nickdesaulniers added inline comments.

================
Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1324
     // This is a string literal initializing an array in an initializer.
-    return CGM.GetConstantArrayFromStringLiteral(E);
+    return E->isLValue() ?
+      CGM.GetAddrOfConstantStringFromLiteral(E).getPointer() :
----------------
efriedma wrote:
> nickdesaulniers wrote:
> > efriedma wrote:
> > > Maybe we should have a separate ConstExprEmitterLValue... trying to 
> > > handle both LValues and RValues on the same codepath has been problematic 
> > > in the past.  It's very easy for code to get confused what it's actually 
> > > trying to emit.
> > So we'd have a `ConstExprEmitterLValue` class with some visitor methods, 
> > and a `ConstExprEmitterRValue` with other methods implemented?
> Something like that.
> 
> Actually thinking about it a bit more, not sure you need to actually 
> implement ConstExprEmitterLValue for now.  You might just be able to ensure 
> we don't ever call ConstExprEmitter with an lvalue.  The current 
> ConstExprEmitter doesn't expect lvalues, and shouldn't call itself with 
> lvalues.  (We bail on explicit LValueToRValue conversions.)  And Evaluate() 
> shouldn't actually evaluate the contents of an lvalue if it isn't 
> dereferenced, so there hopefully aren't any performance issues using that 
> codepath.
> 
> In terms of implementation, I guess that's basically restoring the 
> destType->isReferenceType() that got removed?  (I know I suggested it, but I 
> wasn't really thinking about it...)
One thing I think we may need to add to `ConstExprEmitter` is the ability to 
evaluate `CallExpr`s based on certain test case failures...does that seem right?


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