lebedev.ri added a comment.

In D59467#1432587 <https://reviews.llvm.org/D59467#1432587>, @Tyker wrote:

> i added tests as you requested. i didn't add test for the codegen as it 
> wasn't affected.


Ah right, there wasn't some other clang-specific spelling for this already it 
seems, so indeed codegen needs wiring up too.

Sema tests still missing. (correct usage of attribute in various places with 
different `-std=?` params, incorrect usage of attribute in incorrect places, 
etc)



================
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:
> appear
Please mark fixed comments as fixed (checkbox).


================
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:
> > 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`


================
Comment at: clang/lib/Sema/SemaStmtAttr.cpp:59
+
+  if (!llvm::isa<IfStmt>(St)) {
+    S.Diag(A.getLoc(), diag::err_likely_attr_invalid_placement) << A.getName();
----------------
Where else can it appear?


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