rnk added inline comments.
================ Comment at: clang/lib/AST/ExprConstant.cpp:10166-10182 + EvalInfo Info(Ctx, Result, EvalInfo::EM_ConstantFold); + Info.IsNonTypeTemplateArgument = true; + LValue LV; + if (!EvaluateLValue(this, LV, Info) || Result.HasSideEffects || + !CheckLValueConstantExpression( + Info, getExprLoc(), Ctx.getLValueReferenceType(getType()), LV)) + return false; ---------------- rsmith wrote: > rsmith wrote: > > Neither `EM_ConstantFold` nor `EM_IgnoreSideEffects` seem like the right > > evaluation modes to be using to evaluate a non-type template argument. I > > would expect `EM_ConstantExpression` to be used for both cases. > Oh, I see, we're allowing side-effects here and then issuing a > constant-folding warning elsewhere if there actually were any. *sigh* > > I would expect we can get away with not doing that for template arguments. > It'd be worth testing whether GCC actually allows constant folding there, or > requires a real constant expression. "Allowing side effects" here looks like it really means allowing non-constant subexpressions when those subexpressions would be discarded: ``` template <int *N> struct Foo {}; int x; Foo<(x, &x)> f; constexpr bool is_true = true; int *fn() { if constexpr (((void)x, &x) != nullptr) { return &x; } else { return nullptr; } if constexpr (is_true || x == 2) { return &x; } else { return nullptr; } } ``` Clang accepts this, GCC rejects a few. I'm guessing we want that kind of behavior for `if constexpr` conditions, but maybe not for non-type template parameters. So, it sounds like we should check the CCE kind in SemaOverload, and use a more restrictive constant expression evaluation mode in that case. Perhaps even a new one for non-type template arguments. ================ Comment at: clang/lib/Sema/SemaOverload.cpp:5401 - if ((T->isReferenceType() - ? !Result.get()->EvaluateAsLValue(Eval, S.Context) - : !Result.get()->EvaluateAsRValue(Eval, S.Context)) || + if (!Result.get()->EvaluateAsNonTypeTemplateArgument(Eval, T, S.Context) || (RequireInt && !Eval.Val.isInt())) { ---------------- rsmith wrote: > Don't we get here for `CCEKind`s other than the non-type template argument > case? You're right, but I wasn't able to construct a test case where we would call `CheckConvertedConstantExpression` and we would reject it today because it is dllimported. This was my best idea, using `if constexpr`: ``` struct __declspec(dllimport) Foo { static constexpr bool imported_foo = true; }; const bool some_bool = false; const bool *f() { if constexpr (Foo::imported_foo) { return &Foo::imported_foo; } else { return &some_bool; } } ``` Any other ideas on how to get more coverage of this path through CheckConvertedConstantExpression? https://reviews.llvm.org/D43320 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits