rsmith added inline comments.
================ Comment at: clang/lib/AST/ExprConstant.cpp:4583 if (!InitE) return getDefaultInitValue(VD->getType(), Val); ---------------- The initializer might also be null because the variable is type-dependent (eg, `X<contains_errors> x;`), in which case assuming default-init is wrong. We should check for that and treat it like a value-dependent initializer. ================ Comment at: clang/lib/AST/ExprConstant.cpp:4961 } + if (IS->getCond()->isValueDependent()) + return EvaluateDependentExpr(IS->getCond(), Info); ---------------- hokein wrote: > The `if` stmt (the same to `while`, `for`) is tricky -- if the condition is > value-dependent, then we don't know which branch we should run into. > > - returning a ESR_Succeeded may lead to a bogus diagnostic ("reach the end of > the function"); > - returning a ESR_Failed may lead to a bogus diagnostic ("never produce a > constexpr"); > > I guess what we want is to stop the evaluation, and indicate that we hit a > value-dependent expression and we don't know how to evaluate it, but still > treat the constexpr function as potential constexpr (but no extra diagnostics > being emitted), but the current `EvalStmtResult` is not sufficient, maybe we > need a new enum. We should only produce the "never produce a constant expression" diagnostic if we also produce a CCEDiag/FFDiag, so I think returning ESR_Failed here should work. ================ Comment at: clang/lib/AST/ExprConstant.cpp:4961 } + if (IS->getCond()->isValueDependent()) + return EvaluateDependentExpr(IS->getCond(), Info); ---------------- rsmith wrote: > hokein wrote: > > The `if` stmt (the same to `while`, `for`) is tricky -- if the condition is > > value-dependent, then we don't know which branch we should run into. > > > > - returning a ESR_Succeeded may lead to a bogus diagnostic ("reach the end > > of the function"); > > - returning a ESR_Failed may lead to a bogus diagnostic ("never produce a > > constexpr"); > > > > I guess what we want is to stop the evaluation, and indicate that we hit a > > value-dependent expression and we don't know how to evaluate it, but still > > treat the constexpr function as potential constexpr (but no extra > > diagnostics being emitted), but the current `EvalStmtResult` is not > > sufficient, maybe we need a new enum. > We should only produce the "never produce a constant expression" diagnostic > if we also produce a CCEDiag/FFDiag, so I think returning ESR_Failed here > should work. Should this check live in EvaluateCond instead? ================ Comment at: clang/lib/AST/ExprConstant.cpp:5053 FullExpressionRAII IncScope(Info); if (!EvaluateIgnoredValue(Info, FS->getInc()) || !IncScope.destroy()) return ESR_Failed; ---------------- Missing value dependence check here. ================ Comment at: clang/lib/AST/ExprConstant.cpp:7896 QualType Type = Inner->getType(); - + if (Inner->isValueDependent()) + return EvaluateDependentExpr(Inner, Info); ---------------- How does this happen? I would expect the dependence of the subexpression to be reflected in the MaterializeTemporaryExpr. ================ Comment at: clang/lib/AST/ExprConstant.cpp:8440 } else { + if (SubExpr->isValueDependent()) + return EvaluateDependentExpr(SubExpr, Info); ---------------- How does this happen? ================ Comment at: clang/lib/AST/ExprConstant.cpp:9145 } else if (Init) { + if (Init->isValueDependent()) + return EvaluateDependentExpr(Init, Info); ---------------- How does this happen? Do we not propagate value-dependence from initializers to new-expressions? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84637/new/ https://reviews.llvm.org/D84637 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits