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