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