efriedma 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() :
----------------
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...)


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