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:
> 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
The only things I know of that Evaluate() can't handle are CK_ToUnion,
CK_ReinterpretMemberPointer, and DesignatedInitUpdateExpr.
DesignatedInitUpdateExpr is related to the failures in
test/CodeGenCXX/designated-init.cpp; I don't think the others show up in any of
the testcases you've mentioned. (CK_ToUnion can't appear in C++ code.
CK_ReinterpretMemberPointer is a `reinterpret_cast<T>` where T is a member
pointer type.)
Given none of those constructs show up in const-init-cxx1y.cpp, the only reason
for it to fail is if we aren't correctly falling back for a construct the
current code doesn't know how to handle. You shouldn't need to implement any
new constructs.
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
