efriedma created this revision.
efriedma added a reviewer: rsmith.
Herald added a project: clang.
This allows substantially simplifying the expression evaluation code, because
we don't have to special-case lvalues which are actually string literal
initialization.
This currently throws away an optimization where we would avoid creating an
array APValue for string literal initialization. If we really want to optimize
this case, we should fix APValue so it can store simple arrays more
efficiently, like llvm::ConstantDataArray. This shouldn't affect the memory
usage for other string literals. (Not sure if this is a blocker; I don't think
string literal init is common enough for this to be a serious issue, but I
could be wrong.)
The change to test/CodeGenObjC/encode-test.m is a weird side-effect of these
changes: we currently don't constant-evaluate arrays in C, so the strlen call
shouldn't be folded, but lvalue string init managed to get around that check.
I this this is fine.
Fixes https://bugs.llvm.org/show_bug.cgi?id=40430 .
Repository:
rC Clang
https://reviews.llvm.org/D57935
Files:
lib/AST/ExprConstant.cpp
lib/CodeGen/CGExprConstant.cpp
lib/Sema/SemaInit.cpp
test/AST/ast-dump-wchar.cpp
test/CodeGenObjC/encode-test.m
test/SemaCXX/constant-expression-cxx11.cpp
Index: test/SemaCXX/constant-expression-cxx11.cpp
===================================================================
--- test/SemaCXX/constant-expression-cxx11.cpp
+++ test/SemaCXX/constant-expression-cxx11.cpp
@@ -2220,3 +2220,11 @@
constexpr int *q = (&n + 1) - (unsigned __int128)-1; // expected-error {{constant expression}} expected-note {{cannot refer to element -3402}}
constexpr int *r = &(&n + 1)[(unsigned __int128)-1]; // expected-error {{constant expression}} expected-note {{cannot refer to element 3402}}
}
+
+namespace PR40430 {
+ struct S {
+ char c[10] = "asdf";
+ constexpr char foo() const { return c[3]; }
+ };
+ static_assert(S().foo() == 'f', "");
+}
Index: test/CodeGenObjC/encode-test.m
===================================================================
--- test/CodeGenObjC/encode-test.m
+++ test/CodeGenObjC/encode-test.m
@@ -186,7 +186,8 @@
// CHECK-LABEL: @test_strlen(
// CHECK: %[[i:.*]] = alloca i32
-// CHECK: store i32 1, i32* %[[i]]
+// CHECK: %[[call:.*]] = call i32 @strlen
+// CHECK: store i32 %[[call]], i32* %[[i]]
void test_strlen() {
const char array[] = @encode(int);
int i = strlen(array);
Index: test/AST/ast-dump-wchar.cpp
===================================================================
--- test/AST/ast-dump-wchar.cpp
+++ test/AST/ast-dump-wchar.cpp
@@ -1,13 +1,13 @@
// RUN: %clang_cc1 -std=c++11 -ast-dump %s -triple x86_64-linux-gnu | FileCheck %s
char c8[] = u8"test\0\\\"\a\b\f\n\r\t\v\234";
-// CHECK: StringLiteral {{.*}} lvalue u8"test\000\\\"\a\b\f\n\r\t\v\234"
+// CHECK: StringLiteral {{.*}} u8"test\000\\\"\a\b\f\n\r\t\v\234"
char16_t c16[] = u"test\0\\\"\t\a\b\234\u1234";
-// CHECK: StringLiteral {{.*}} lvalue u"test\000\\\"\t\a\b\234\u1234"
+// CHECK: StringLiteral {{.*}} u"test\000\\\"\t\a\b\234\u1234"
char32_t c32[] = U"test\0\\\"\t\a\b\234\u1234\U0010ffff"; // \
-// CHECK: StringLiteral {{.*}} lvalue U"test\000\\\"\t\a\b\234\u1234\U0010FFFF"
+// CHECK: StringLiteral {{.*}} U"test\000\\\"\t\a\b\234\u1234\U0010FFFF"
wchar_t wc[] = L"test\0\\\"\t\a\b\234\u1234\xffffffff"; // \
-// CHECK: StringLiteral {{.*}} lvalue L"test\000\\\"\t\a\b\234\x1234\xFFFFFFFF"
+// CHECK: StringLiteral {{.*}} L"test\000\\\"\t\a\b\234\x1234\xFFFFFFFF"
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -144,6 +144,7 @@
static void updateStringLiteralType(Expr *E, QualType Ty) {
while (true) {
E->setType(Ty);
+ E->setValueKind(VK_RValue);
if (isa<StringLiteral>(E) || isa<ObjCEncodeExpr>(E))
break;
else if (ParenExpr *PE = dyn_cast<ParenExpr>(E))
Index: lib/CodeGen/CGExprConstant.cpp
===================================================================
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1649,16 +1649,6 @@
llvm::Constant *ConstantLValueEmitter::tryEmit() {
const APValue::LValueBase &base = Value.getLValueBase();
- // Certain special array initializers are represented in APValue
- // as l-values referring to the base expression which generates the
- // array. This happens with e.g. string literals. These should
- // probably just get their own representation kind in APValue.
- if (DestType->isArrayType()) {
- assert(!hasNonZeroOffset() && "offset on array initializer");
- auto expr = const_cast<Expr*>(base.get<const Expr*>());
- return ConstExprEmitter(Emitter).Visit(expr, DestType);
- }
-
// Otherwise, the destination type should be a pointer or reference
// type, but it might also be a cast thereof.
//
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -2688,9 +2688,11 @@
}
// Expand a string literal into an array of characters.
-static void expandStringLiteral(EvalInfo &Info, const Expr *Lit,
+//
+// FIXME: This is inefficient; we should probably introduce something similar
+// to the LLVM ConstantDataArray to make this cheaper.
+static void expandStringLiteral(EvalInfo &Info, const StringLiteral *S,
APValue &Result) {
- const StringLiteral *S = cast<StringLiteral>(Lit);
const ConstantArrayType *CAT =
Info.Ctx.getAsConstantArrayType(S->getType());
assert(CAT && "string literal isn't an array");
@@ -2884,18 +2886,6 @@
ObjType = CAT->getElementType();
- // An array object is represented as either an Array APValue or as an
- // LValue which refers to a string literal.
- if (O->isLValue()) {
- assert(I == N - 1 && "extracting subobject of character?");
- assert(!O->hasLValuePath() || O->getLValuePath().empty());
- if (handler.AccessKind != AK_Read)
- expandStringLiteral(Info, O->getLValueBase().get<const Expr *>(),
- *O);
- else
- return handler.foundString(*O, ObjType, Index);
- }
-
if (O->getArrayInitializedElts() > Index)
O = &O->getArrayInitializedElt(Index);
else if (handler.AccessKind != AK_Read) {
@@ -3008,11 +2998,6 @@
Result = APValue(Value);
return true;
}
- bool foundString(APValue &Subobj, QualType SubobjType, uint64_t Character) {
- Result = APValue(extractStringLiteralCharacter(
- Info, Subobj.getLValueBase().get<const Expr *>(), Character));
- return true;
- }
};
} // end anonymous namespace
@@ -3070,9 +3055,6 @@
Value = NewVal.getFloat();
return true;
}
- bool foundString(APValue &Subobj, QualType SubobjType, uint64_t Character) {
- llvm_unreachable("shouldn't encounter string elements with ExpandArrays");
- }
};
} // end anonymous namespace
@@ -3386,12 +3368,20 @@
CompleteObject LitObj(&Lit, Base->getType(), false);
return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal);
} else if (isa<StringLiteral>(Base) || isa<PredefinedExpr>(Base)) {
- // We represent a string literal array as an lvalue pointing at the
- // corresponding expression, rather than building an array of chars.
- // FIXME: Support ObjCEncodeExpr, MakeStringConstant
- APValue Str(Base, CharUnits::Zero(), APValue::NoLValuePath(), 0);
- CompleteObject StrObj(&Str, Base->getType(), false);
- return extractSubobject(Info, Conv, StrObj, LVal.Designator, RVal);
+ // Special-case character extraction so we don't have to construct an
+ // APValue for the whole string.
+ assert(LVal.Designator.Entries.size() == 1 &&
+ "Can only read characters from string literals");
+ if (LVal.Designator.isOnePastTheEnd()) {
+ if (Info.getLangOpts().CPlusPlus11)
+ Info.FFDiag(Conv, diag::note_constexpr_access_past_end) << AK_Read;
+ else
+ Info.FFDiag(Conv);
+ return false;
+ }
+ uint64_t CharIndex = LVal.Designator.Entries[0].ArrayIndex;
+ RVal = APValue(extractStringLiteralCharacter(Info, Base, CharIndex));
+ return true;
}
}
@@ -3517,9 +3507,6 @@
LVal.moveInto(Subobj);
return true;
}
- bool foundString(APValue &Subobj, QualType SubobjType, uint64_t Character) {
- llvm_unreachable("shouldn't encounter string elements here");
- }
};
} // end anonymous namespace
@@ -3668,9 +3655,6 @@
LVal.moveInto(Subobj);
return true;
}
- bool foundString(APValue &Subobj, QualType SubobjType, uint64_t Character) {
- llvm_unreachable("shouldn't encounter string elements here");
- }
};
} // end anonymous namespace
@@ -7150,8 +7134,7 @@
: ExprEvaluatorBaseTy(Info), This(This), Result(Result) {}
bool Success(const APValue &V, const Expr *E) {
- assert((V.isArray() || V.isLValue()) &&
- "expected array or string literal");
+ assert(V.isArray() && "expected array");
Result = V;
return true;
}
@@ -7182,6 +7165,10 @@
bool VisitCXXConstructExpr(const CXXConstructExpr *E,
const LValue &Subobject,
APValue *Value, QualType Type);
+ bool VisitStringLiteral(const StringLiteral *E) {
+ expandStringLiteral(Info, E, Result);
+ return true;
+ }
};
} // end anonymous namespace
@@ -7214,14 +7201,8 @@
// C++11 [dcl.init.string]p1: A char array [...] can be initialized by [...]
// an appropriately-typed string literal enclosed in braces.
- if (E->isStringLiteralInit()) {
- LValue LV;
- if (!EvaluateLValue(E->getInit(0), LV, Info))
- return false;
- APValue Val;
- LV.moveInto(Val);
- return Success(Val, E);
- }
+ if (E->isStringLiteralInit())
+ return Visit(E->getInit(0));
bool Success = true;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
