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
