[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 22 06:15:28 PDT 2020
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
More information about the cfe-commits
mailing list