aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3696-3697
+
+ if (E->isValueDependent() || E->isTypeDependent())
+ continue;
+
----------------
Should this move up above the point where we're checking whether the expression
has an array type?
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3706
+
+ if (!Result || !Notes.empty()) {
+ Diag(E->getBeginLoc(), diag::err_attribute_argument_n_type)
----------------
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?
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3693
+ if (E->isValueDependent() || E->isTypeDependent() ||
+ (CE && CE->hasAPValueResult()))
+ continue;
----------------
Tyker wrote:
> aaron.ballman wrote:
> > What is the rationale for bailing out when the constant expression has an
> > `APValue` result?
> the intent was that during nested template instantiation arguement will be
> evaluated as soon as they are neither value nor type dependent and the result
> will be stored in the ConstantExpr. if an arguement has already been
> evaluated in a previous instantiation we don't want to evaluate it again.
> but because of SubstExpr ConstantExpr are getting removed so it removed it.
Ah, thank you for the explanation and change.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:195
+ return;
+ else
+ Args.push_back(Result.get());
----------------
No `else` after a `return`.
================
Comment at: clang/test/SemaCXX/attr-annotate.cpp:96
+}
+
+void test() {}
----------------
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?
```
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
