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