r353569 - [Sema] Make string literal init an rvalue.

Eli Friedman via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 8 18:23:28 PST 2019


Fixed in r353598.

-Eli

> -----Original Message-----
> From: cfe-commits <cfe-commits-bounces at lists.llvm.org> On Behalf Of Eli
> Friedman via cfe-commits
> Sent: Friday, February 8, 2019 6:06 PM
> To: douglas.yung at sony.com
> Cc: cfe-commits at lists.llvm.org
> Subject: [EXT] RE: r353569 - [Sema] Make string literal init an rvalue.
> 
> Looking.  It looks like this only fails with clang-tidy, so I'll give myself a few
> minutes to look before reverting.
> 
> -Eli
> 
> > -----Original Message-----
> > From: douglas.yung at sony.com <douglas.yung at sony.com>
> > Sent: Friday, February 8, 2019 3:58 PM
> > To: Eli Friedman <efriedma at quicinc.com>
> > Cc: cfe-commits at lists.llvm.org
> > Subject: [EXT] RE: r353569 - [Sema] Make string literal init an rvalue.
> >
> > Hi Eli,
> >
> > Your commit is causing a failure on the PS4 linux bot, can you please take a
> > look?
> >
> > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-
> > fast/builds/43625
> >
> > FAIL: Clang Tools :: clang-tidy/bugprone-string-constructor.cpp (14325 of
> > 46835)
> > ******************** TEST 'Clang Tools :: clang-tidy/bugprone-string-
> > constructor.cpp' FAILED ********************
> > Script:
> > --
> > : 'RUN: at line 1';   /usr/bin/python2.7 /home/buildslave/ps4-buildslave4/llvm-
> > clang-lld-x86_64-scei-ps4-ubuntu-
> > fast/llvm.src/tools/clang/tools/extra/test/../test/clang-
> > tidy/check_clang_tidy.py /home/buildslave/ps4-buildslave4/llvm-clang-lld-
> > x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-
> > tidy/bugprone-string-constructor.cpp bugprone-string-constructor
> > /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-
> > fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/bugprone-string-
> > constructor.cpp.tmp
> > --
> > Exit Code: 1
> >
> > Command Output (stdout):
> > --
> > Running ['clang-tidy', '/home/buildslave/ps4-buildslave4/llvm-clang-lld-
> x86_64-
> > scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-
> > tidy/Output/bugprone-string-constructor.cpp.tmp.cpp', '-fix', '--checks=-
> > *,bugprone-string-constructor', '--', '--std=c++11', '-nostdinc++']...
> > clang-tidy failed:
> > clang-tidy: /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-
> > ubuntu-fast/llvm.src/tools/clang/lib/AST/ExprConstant.cpp:3374: bool
> > handleLValueToRValueConversion((anonymous namespace)::EvalInfo &, const
> > clang::Expr *, clang::QualType, const (anonymous namespace)::LValue &,
> > clang::APValue &): Assertion `LVal.Designator.Entries.size() == 1 && "Can only
> > read characters from string literals"' failed.
> >  #0 0x00000000004ad2c4 (clang-tidy+0x4ad2c4)
> >  #1 0x00000000004ab03c (clang-tidy+0x4ab03c)
> >  #2 0x00000000004ad878 (clang-tidy+0x4ad878)
> >  #3 0x00007f034560b890 __restore_rt (/lib/x86_64-linux-
> > gnu/libpthread.so.0+0x12890)
> >  #4 0x00007f03442d1e97 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x3ee97)
> >  #5 0x00007f03442d3801 abort (/lib/x86_64-linux-gnu/libc.so.6+0x40801)
> >  #6 0x00007f03442c339a (/lib/x86_64-linux-gnu/libc.so.6+0x3039a)
> >  #7 0x00007f03442c3412 (/lib/x86_64-linux-gnu/libc.so.6+0x30412)
> >  #8 0x0000000001941c9d (clang-tidy+0x1941c9d)
> >  #9 0x000000000192b797 (clang-tidy+0x192b797)
> > #10 0x00000000019266be (clang-tidy+0x19266be)
> > #11 0x00000000005771f9 (clang-tidy+0x5771f9)
> > #12 0x0000000001769d11 (clang-tidy+0x1769d11)
> > #13 0x0000000001782d4b (clang-tidy+0x1782d4b)
> > #14 0x0000000001769497 (clang-tidy+0x1769497)
> > #15 0x0000000001774613 (clang-tidy+0x1774613)
> > #16 0x000000000177161f (clang-tidy+0x177161f)
> > #17 0x0000000001782ad2 (clang-tidy+0x1782ad2)
> > #18 0x000000000176b3e6 (clang-tidy+0x176b3e6)
> > #19 0x000000000177f17d (clang-tidy+0x177f17d)
> > #20 0x000000000177161f (clang-tidy+0x177161f)
> > #21 0x00000000017829f3 (clang-tidy+0x17829f3)
> > #22 0x000000000176b0bd (clang-tidy+0x176b0bd)
> > #23 0x000000000176e8b7 (clang-tidy+0x176e8b7)
> > #24 0x000000000176cfae (clang-tidy+0x176cfae)
> > #25 0x000000000174981f (clang-tidy+0x174981f)
> > #26 0x0000000000c5020c (clang-tidy+0xc5020c)
> > #27 0x0000000000d98873 (clang-tidy+0xd98873)
> > #28 0x0000000000c381c0 (clang-tidy+0xc381c0)
> > #29 0x0000000000bdcd31 (clang-tidy+0xbdcd31)
> > #30 0x0000000000834386 (clang-tidy+0x834386)
> > #31 0x00000000004ccba5 (clang-tidy+0x4ccba5)
> > #32 0x00000000008340f6 (clang-tidy+0x8340f6)
> > #33 0x0000000000833997 (clang-tidy+0x833997)
> > #34 0x000000000083579c (clang-tidy+0x83579c)
> > #35 0x00000000004c9505 (clang-tidy+0x4c9505)
> > #36 0x000000000041d427 (clang-tidy+0x41d427)
> > #37 0x00007f03442b4b97 __libc_start_main (/lib/x86_64-linux-
> > gnu/libc.so.6+0x21b97)
> > #38 0x000000000041b2fa (clang-tidy+0x41b2fa)
> >
> >
> > --
> > Command Output (stderr):
> > --
> > Traceback (most recent call last):
> >   File "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-
> ubuntu-
> > fast/llvm.src/tools/clang/tools/extra/test/../test/clang-
> > tidy/check_clang_tidy.py", line 207, in <module>
> >     main()
> >   File "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-
> ubuntu-
> > fast/llvm.src/tools/clang/tools/extra/test/../test/clang-
> > tidy/check_clang_tidy.py", line 147, in main
> >     subprocess.check_output(args, stderr=subprocess.STDOUT).decode()
> >   File "/usr/lib/python2.7/subprocess.py", line 223, in check_output
> >     raise CalledProcessError(retcode, cmd, output=output)
> > subprocess.CalledProcessError: Command '['clang-tidy',
> '/home/buildslave/ps4-
> > buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-
> > fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/bugprone-string-
> > constructor.cpp.tmp.cpp', '-fix', '--checks=-*,bugprone-string-constructor', '--',
> > '--std=c++11', '-nostdinc++']' returned non-zero exit status -6
> >
> > Douglas Yung
> >
> > -----Original Message-----
> > From: cfe-commits <cfe-commits-bounces at lists.llvm.org> On Behalf Of Eli
> > Friedman via cfe-commits
> > Sent: Friday, February 8, 2019 13:19
> > To: cfe-commits at lists.llvm.org
> > Subject: r353569 - [Sema] Make string literal init an rvalue.
> >
> > Author: efriedma
> > Date: Fri Feb  8 13:18:46 2019
> > New Revision: 353569
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=353569&view=rev
> > Log:
> > [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 .
> >
> >
> > Modified:
> >     cfe/trunk/lib/AST/ExprConstant.cpp
> >     cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> >     cfe/trunk/lib/Sema/SemaInit.cpp
> >     cfe/trunk/test/AST/ast-dump-wchar.cpp
> >     cfe/trunk/test/CodeGenObjC/encode-test.m
> >     cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
> >
> > Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> > URL: http://llvm.org/viewvc/llvm-
> >
> project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=353569&r1=353568&r2=35356
> > 9&view=diff
> >
> =================================================================
> > =============
> > --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> > +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Feb  8 13:18:46 2019
> > @@ -2688,9 +2688,11 @@ static APSInt extractStringLiteralCharac  }
> >
> >  // 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
> >
> >        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 handleLValueToRValueConversi
> >        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::VisitInitListEx
> >
> >    // 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;
> >
> >
> > Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> > URL: http://llvm.org/viewvc/llvm-
> >
> project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=353569&r1=353568&r
> > 2=353569&view=diff
> >
> =================================================================
> > =============
> > --- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Fri Feb  8 13:18:46 2019
> > @@ -1649,16 +1649,6 @@ private:
> >  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.
> >    //
> >
> > Modified: cfe/trunk/lib/Sema/SemaInit.cpp
> > URL: http://llvm.org/viewvc/llvm-
> >
> project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=353569&r1=353568&r2=353569&
> > view=diff
> >
> =================================================================
> > =============
> > --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Feb  8 13:18:46 2019
> > @@ -144,6 +144,7 @@ static StringInitFailureKind IsStringIni  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))
> >
> > Modified: cfe/trunk/test/AST/ast-dump-wchar.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-
> > wchar.cpp?rev=353569&r1=353568&r2=353569&view=diff
> >
> =================================================================
> > =============
> > --- cfe/trunk/test/AST/ast-dump-wchar.cpp (original)
> > +++ cfe/trunk/test/AST/ast-dump-wchar.cpp Fri Feb  8 13:18:46 2019
> > @@ -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"
> >
> > Modified: cfe/trunk/test/CodeGenObjC/encode-test.m
> > URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/test/CodeGenObjC/encode-
> > test.m?rev=353569&r1=353568&r2=353569&view=diff
> >
> =================================================================
> > =============
> > --- cfe/trunk/test/CodeGenObjC/encode-test.m (original)
> > +++ cfe/trunk/test/CodeGenObjC/encode-test.m Fri Feb  8 13:18:46 2019
> > @@ -186,7 +186,8 @@ size_t strlen(const char *s);
> >
> >  // 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);
> >
> > Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-
> > expression-cxx11.cpp?rev=353569&r1=353568&r2=353569&view=diff
> >
> =================================================================
> > =============
> > --- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original)
> > +++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Fri Feb  8
> > +++ 13:18:46 2019
> > @@ -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', "");
> > +}
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


More information about the cfe-commits mailing list