[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> Indeed, but I wonder why only do this when IsForRef == true (narrator: it's not)?

A MaterializeTemporaryExpr is an lvalue; it only makes sense in lvalue contexts.

> Also, why for MaterializeTemporaryExpr manually force the ForRef parameter to false?

The operand to a MaterializeTemporaryExpr is an rvalue.

> 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.

I think we want to keep lvalue and rvalue handling separate, for the same reasons we want to keep it separate in other places.  It's very easy to get the semantics wrong if you try to handle lvalues and rvalues in the same switch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151587