aaron.ballman added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:4921
+ // value is.
+ if (isa<RecoveryExpr>(E))
+ return false;
----------------
yronglin wrote:
> hokein wrote:
> > The constant evaluator is not aware of the "error" concept, it is only
> > aware of value-dependent -- the general idea behind is that we treat the
> > dependent-on-error and dependent-on-template-parameters cases the same,
> > they are potentially constant (if we see an expression contains errors, it
> > could be constant depending on how the error is resolved), this will give
> > us nice recovery and avoid bogus following diagnostics.
> >
> > So, a `RecoveryExpr` should not result in failure when checking for a
> > potential constant expression.
> >
> > I think the right fix is to remove the conditional check `if
> > (!EvaluateDependentExpr(SS->getCond(), Info))` in `EvaluateSwitch`, and
> > return `ESR_Failed` unconditionally (we don't know its value, any
> > switch-case anwser will be wrong in some cases). We already do this for
> > return-statment, do-statement etc.
> >
> >
> Do you mean?
> ```
> if (SS->getCond()->isValueDependent()) {
> EvaluateDependentExpr(SS->getCond(), Info);
> return ESR_Failed;
> }
> ```
> the general idea behind is that we treat the dependent-on-error and
> dependent-on-template-parameters cases the same, they are potentially
> constant (if we see an expression contains errors, it could be constant
> depending on how the error is resolved), this will give us nice recovery and
> avoid bogus following diagnostics.
I could use some further education on why this is the correct approach. For a
dependent-on-template-parameters case, this makes sense -- either the template
will be instantiated (at which point we'll know if it's a constant expression)
or it won't be (at which point it's constant expression-ness doesn't matter).
But for error recovery, we will *never* get a valid constant expression.
I worry about the performance overhead of continuing on with constant
expression evaluation in the error case. We use these code paths not only to
get a value but to say "is this a constant expression at all?".
I don't see why the fix should be localized to just the switch statement
condition; it seems like *any* attempt to get a dependent value from an error
recovery expression is a point at which we can definitively say "this is not a
constant expression" and move on.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153296/new/
https://reviews.llvm.org/D153296
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
