r353569 - [Sema] Make string literal init an rvalue.
Eli Friedman via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 8 18:05:46 PST 2019
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
More information about the cfe-commits
mailing list