aaron.ballman added inline comments.

================
Comment at: clang/include/clang/Basic/Attr.td:1151
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201903>, CXX11<"clang", "likely">];
+  let Documentation = [LikelyDocs];
----------------
I think this should have a C spelling as well, so I'd probably go with: 
`[CXX11<"", "likely", 201903L>, Clang<"likely">]` and then similar for 
`unlikely`. I would probably use the same semantic attribute for both (so add 
both spellings and an accessor to differentiate). Rather than naming the 
attribute `Likely`, I would go with `Likelihood` so it covers both spellings 
more naturally.


================
Comment at: clang/include/clang/Basic/Attr.td:1153
+  let Documentation = [LikelyDocs];
+}
+
----------------
Can you add a commented-out `Subjects` that take `Stmt` and `LabelStmt`? It's 
not useful now, but I still hope that someday we can tablegen more checks for 
statement and type attributes like we already do for declarations.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8164
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't apear here">;
+
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > appear
> > Please mark fixed comments as fixed (checkbox).
> 'here'?
> Would be nicer to be more explanatory.
> `likely annotation can't appear on %0; can only appear on x, y, x`
This should follow the pattern used by decl attributes:
```
def err_attribute_wrong_stmt_type : Error<
  "%0 attribute only applies to statements and labels">;
```
There may be a diagnostic used for `[[fallthrough]]` that could be combined 
with this diagnostic, since that attribute can only be applied to null 
statements.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59467/new/

https://reviews.llvm.org/D59467



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to