[PATCH] D84637: [AST] Enhance the const expression evaluator to support error-dependent exprs.
Richard Smith - zygoloid via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 8 13:04:40 PDT 2020
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
More information about the cfe-commits
mailing list