nickdesaulniers added a comment.

In D151587#4516772 <https://reviews.llvm.org/D151587#4516772>, @efriedma wrote:

> This seems to work:
>
>   diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
>   index 25d3535e..98b1e4d 100644
>   --- a/clang/lib/AST/Expr.cpp
>   +++ b/clang/lib/AST/Expr.cpp
>   @@ -3319,6 +3319,10 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, 
> bool IsForRef,
>      // kill the second parameter.
>   
>      if (IsForRef) {
>   +    if (auto *EWC = dyn_cast<ExprWithCleanups>(this))
>   +      return EWC->getSubExpr()->isConstantInitializer(Ctx, true, Culprit);
>   +    if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(this))
>   +      return MTE->getSubExpr()->isConstantInitializer(Ctx, false, Culprit);

Indeed, but I wonder why only do this when `IsForRef == true` (narrator: it's 
not)? Also, why for `MaterializeTemporaryExpr` manually force the `ForRef` 
parameter to `false`?

---

It seems like perhaps we want to call `EvaluateAsLValue` in 
`Expr::isConstantInitializer` AFTER checking the statement class.  Otherwise 
we're basically reimplementing that big switch statement again; at least two 
cases as suggested by your diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151587

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to