[PATCH] D62009: [clang] perform semantic checking in constant context

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D62009#1506415 <https://reviews.llvm.org/D62009#1506415>, @Tyker wrote:

> there are issues with using ConstantExpr to mark expressions as constant for semantic checking:
>
> - #1 multpile Expr::Ignore* operation remove ConstantExpr from the expression.


`Ignore*` is generally used when walking the syntactic form of an expression, so this is usually appropriate.

> - #2 Semantic checking Traverses the AST so all methods that only mark the top-level Node will not completely work.

Semantic checks that care about constant evaluation should not walk over a `ConstantExpr` node. Generally, we want rather different checking inside a `ConstantExpr` node than outside (for example, we shouldn't be performing any checks for potential overflow or narrowing or conversions, and instead should be finding out what *actually* happens by evaluating the expression and diagnosing problems in it). So this seems fine too, though there may still be places that need updates to properly handle `ConstantExpr`.

> - #3 at short term: adding ConstantExpr modifies the AST structure, so adding it everywhere it is needed for semantic checking will require changing code that depend closely on the AST structure.

Yes. But that's going to be the case for any approach we take here.

> - push a ExpressionEvaluationContext::ConstantEvaluated so Sema will propagate it and propagate from Sema to the expression evaluator via boolean values.
> - put all semantic checking function's in a SemaCheckContext class and propagate via this class to the expression evaluator. this class will be usable to propagate Sema and probably other informations.
> - keep the bit in ExprBitfield but make all nodes created in ExpressionEvaluationContext::ConstantEvaluated marked for constant evaluation to fixes limitation #2.

We don't always know whether an expression is constant evaluated until after we've parsed it (eg, for a call to a function that might be `consteval` we need to perform overload resolution before we find out). And this would require changing all the (many) places that create `Expr` nodes to pass in new information. That sounds unpleasant and expensive, and adds a permanent cost to all future AST changes.

So far we don't appear to need any per-`Expr` indicator of whether that expression is transitively within a `ConstantExpr`, so let's not add that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62009/new/

https://reviews.llvm.org/D62009