aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM aside from a request for a comment to be added. Thank you! ================ Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3706 + + if (!Result || !Notes.empty()) { + Diag(E->getBeginLoc(), diag::err_attribute_argument_n_type) ---------------- Tyker wrote: > aaron.ballman wrote: > > I'm surprised that the presence of notes alone would mean the attribute > > argument has an error and should be skipped. Are there circumstances under > > which `Result` is true but `Notes` is non-empty? > AFAIK > Result means the expression can be folded to a constant. > Note.empty() means the expression is a valid constant expression in the > current language mode. > > the difference is observable in code like: > ``` > constexpr int foldable_but_invalid() { > int *A = new int(0); > return *A; > } > > [[clang::annotate("", foldable_but_invalid())]] void f1() {} > ``` > foldable_but_invalid retruns a constant but any constant evaluation of this > function is invalid because it doesn't desallocate A. > with !Notes.empty() this fails, without it no errors occurs. > > i think it is desirable that attributes don't diverge from the language mode. > I added a test for this. Ah, thank you for explaining that! Can you add a comment to the code to make that clear for others who may run across it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88645/new/ https://reviews.llvm.org/D88645 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits