aaron.ballman added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:13547-13551
+ if (auto ValD = Info.EvaluatingDecl.dyn_cast<const ValueDecl *>()) {
+ const VarDecl *VD = dyn_cast_or_null<VarDecl>(ValD);
+ if (VD && !VD->isConstexpr())
+ NotConstexprVar = true;
+ }
----------------
This seems to be equivalent unless I'm misunderstanding something about the
member dyn_cast.
================
Comment at: clang/lib/AST/ExprConstant.cpp:13572-13575
if (ED->getNumNegativeBits() &&
(Max.slt(Result.getInt().getSExtValue()) ||
- Min.sgt(Result.getInt().getSExtValue())))
- Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
-
diag::warn_constexpr_unscoped_enum_out_of_range)
- << llvm::toString(Result.getInt(),10) << Min.getSExtValue() <<
Max.getSExtValue();
+ Min.sgt(Result.getInt().getSExtValue())) &&
+ !NotConstexprVar)
----------------
Might as well test the easy condition before doing more work to get values and
compare them.
I think we should rename `NotConstexprVar` to `ConstexprVar` so that we don't
need the double negation here. WDYT?
================
Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:2424-2427
+E2 testDefaultArgForParam(E2 e2Param = (E2)-1) { // ok, not a constant
expression context
+ E2 e2LocalInit = e2Param; // ok, not a constant expression context
+ return e2LocalInit;
+}
----------------
Do you think it's worth it to add this as a test as well?
```
consteval void testDefaultArgForParam2(E2 e2Param = (E2)-1) {
}
void test() {
testDefaultArgForParam2(); // Make sure we get the error
testDefaultArgForParam2((E2)0); // Make sure we don't get the error
}
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131874/new/
https://reviews.llvm.org/D131874
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
