[Sema] Make string literal init an rvalue.

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 .

llvm-svn: 353569

Author: efriedma-quic

clang/lib/AST/ExprConstant.cpp

@@ -2688,9 +2688,11 @@ static APSInt extractStringLiteralCharacter(EvalInfo &Info, const Expr *Lit,
 }
 
 // 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 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
 
       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 @@ struct ExtractSubobjectHandler {
     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 @@ struct ModifySubobjectHandler {
     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 @@ static bool handleLValueToRValueConversion(EvalInfo &Info, const Expr *Conv,
       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 @@ struct CompoundAssignSubobjectHandler {
     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 @@ struct IncDecSubobjectHandler {
     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 @@ namespace {
       : 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 @@ namespace {
     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 @@ bool ArrayExprEvaluator::VisitInitListExpr(const InitListExpr *E) {
 
   // 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;
 

clang/lib/CodeGen/CGExprConstant.cpp

@@ -1649,16 +1649,6 @@ class ConstantLValueEmitter : public ConstStmtVisitor<ConstantLValueEmitter,
 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.
   //

clang/lib/Sema/SemaInit.cpp

@@ -144,6 +144,7 @@ static StringInitFailureKind IsStringInit(Expr *init, QualType declType,
 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))

clang/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"

clang/test/CodeGenObjC/encode-test.m

@@ -186,7 +186,8 @@ @implementation Derived1X @end
 
 // 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);

clang/test/SemaCXX/constant-expression-cxx11.cpp

@@ -2220,3 +2220,11 @@ namespace PointerArithmeticOverflow {
   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', "");
+}