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() :
----------------
nickdesaulniers wrote:
> 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?
See also the calls to `constexpr f()` in 
clang/test/CodeGenCXX/const-init-cxx1y.cpp


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