Fixed in r353598. -Eli
> -----Original Message----- > From: cfe-commits <cfe-commits-boun...@lists.llvm.org> On Behalf Of Eli > Friedman via cfe-commits > Sent: Friday, February 8, 2019 6:06 PM > To: douglas.y...@sony.com > Cc: cfe-commits@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.y...@sony.com <douglas.y...@sony.com> > > Sent: Friday, February 8, 2019 3:58 PM > > To: Eli Friedman <efrie...@quicinc.com> > > Cc: cfe-commits@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-boun...@lists.llvm.org> On Behalf Of Eli > > Friedman via cfe-commits > > Sent: Friday, February 8, 2019 13:19 > > To: cfe-commits@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@lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
