[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.

[Tyker via Phabricator via llvm-commits]

Tyker added inline comments.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3706
+
+    if (!Result || !Notes.empty()) {
+      Diag(E->getBeginLoc(), diag::err_attribute_argument_n_type)
----------------
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.


================
Comment at: clang/test/SemaCXX/attr-annotate.cpp:96
+}
+
+void test() {}
----------------
aaron.ballman wrote:
> Some more tests for various constant expression situations that may be weird:
> ```
> int n = 10;
> int vla[n];
> 
> [[clang::annotate("vlas are awful", sizeof(vla[++n]))] int i = 0; // reject, the sizeof is not unevaluated
> 
> [[clang::annotate("_Generic selection expression should be fine", _Generic(n, int : 0, default : 1))]] int j = 0; // second arg should resolve to 0 fine
> 
> void designator();
> [[clang::annotate("function designators?", designator)]] int k = 0; // Should work?
> ```
added CK_FunctionToPointerDecay cast when needed
the rest was working as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88645