Fix/interp init list unnamed bitfields #87799
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, types like this one would be mis-initialized:
```c++
struct S {
unsigned : 12;
unsigned f : 8;
}
constexpr S s = { 1 };
```
Because, although the unnamed bit-field has an index in the Record, it
does not actually take any initializer values from an expression such
as the above.
Checks some of the different init expressions permitted in C-land (but not C++).
Is it ever possible to sneak mismatched types past Sema far enough to get to this point? These assert that it is not.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: None (sethp) ChangesPreviously, types like this one would be mis-initialized: struct S {
unsigned : 12;
unsigned f : 8;
}
constexpr S s = { 1 };Because, although the unnamed bit-field has an index in the Record, it This one's for @tbaederr: it's broken out from the work I was doing experimenting with the bit-field bit-casting tests, since it seems mostly self-contained. While separating it out, I had the question of "will iterating the Inits in order always work?": it will for C++, I think, but I'm not so sure about C. I wrote a test to find out (63587ae), and I manually-fuzzed my way into a failure for an unrelated reason that seems C-specific (a similar construct in I was curious what you'd like to do: hold here awaiting a fix for the underlying issue? Pull the failing Thanks! Full diff: https://github.com/llvm/llvm-project/pull/87799.diff 3 Files Affected:
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 46182809810bcf..bf7752d78ae579 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -884,8 +884,36 @@ bool ByteCodeExprGen<Emitter>::visitInitList(ArrayRef<const Expr *> Inits,
if (!this->emitDupPtr(E))
return false;
+ // guard relatively expensive base check behind an almost-always-false
+ // branch. this works because all bases must be initialized first before we
+ // initialize any direct fields
+ if (InitIndex == 0) {
+ // Initializer for a direct base class?
+ if (const Record::Base *B = R->getBase(Init->getType())) {
+ if (!this->emitGetPtrBasePop(B->Offset, Init))
+ return false;
+
+ if (!this->visitInitializer(Init))
+ return false;
+
+ if (!this->emitFinishInitPop(E))
+ return false;
+ // Base initializers don't increase InitIndex, since they don't count
+ // into the Record's fields.
+ continue;
+ }
+ } else {
+ assert(!R->getBase(Init->getType()));
+ }
+
+ // skip over padding-only fields
+ while (R->getField(InitIndex)->Decl->isUnnamedBitfield())
+ ++InitIndex;
+
+ const Record::Field *FieldToInit = R->getField(InitIndex++);
if (std::optional<PrimType> T = classify(Init)) {
- const Record::Field *FieldToInit = R->getField(InitIndex);
+ assert(classifyPrim(FieldToInit->Decl->getType()) == *T);
+
if (!this->visit(Init))
return false;
@@ -899,34 +927,25 @@ bool ByteCodeExprGen<Emitter>::visitInitList(ArrayRef<const Expr *> Inits,
if (!this->emitPopPtr(E))
return false;
- ++InitIndex;
} else {
- // Initializer for a direct base class.
- if (const Record::Base *B = R->getBase(Init->getType())) {
- if (!this->emitGetPtrBasePop(B->Offset, Init))
- return false;
-
- if (!this->visitInitializer(Init))
- return false;
-
- if (!this->emitFinishInitPop(E))
- return false;
- // Base initializers don't increase InitIndex, since they don't count
- // into the Record's fields.
- } else {
- const Record::Field *FieldToInit = R->getField(InitIndex);
- // Non-primitive case. Get a pointer to the field-to-initialize
- // on the stack and recurse into visitInitializer().
- if (!this->emitGetPtrField(FieldToInit->Offset, Init))
- return false;
+ assert(!Init->getType()->isRecordType() ||
+ getRecord(FieldToInit->Decl->getType()) ==
+ getRecord(Init->getType()));
+ assert(
+ !Init->getType()->isArrayType() ||
+ (FieldToInit->Decl->getType()->isArrayType() &&
+ Init->getType()->getArrayElementTypeNoTypeQual() ==
+ FieldToInit->Decl->getType()->getArrayElementTypeNoTypeQual()));
+ // Non-primitive case. Get a pointer to the field-to-initialize
+ // on the stack and recurse into visitInitializer().
+ if (!this->emitGetPtrField(FieldToInit->Offset, Init))
+ return false;
- if (!this->visitInitializer(Init))
- return false;
+ if (!this->visitInitializer(Init))
+ return false;
- if (!this->emitPopPtr(E))
- return false;
- ++InitIndex;
- }
+ if (!this->emitPopPtr(E))
+ return false;
}
}
return true;
diff --git a/clang/test/AST/Interp/records.c b/clang/test/AST/Interp/records.c
new file mode 100644
index 00000000000000..1492fafae2eba7
--- /dev/null
+++ b/clang/test/AST/Interp/records.c
@@ -0,0 +1,114 @@
+// UNSUPPORTED: asserts
+// REQUIRES: asserts
+// ^ this attempts to say "don't actually run this test", because it's broken
+//
+// The point of this test is to demonstrate something that ExprConstant accepts,
+// but Interp rejects. I had hoped to express that as the same file with two
+// sets of RUNs: one for the classic evaluator, which would be expected to
+// succeed, and one for the new interpreter which would be expected to fail (so
+// the overall test passes just in case the new interpreter rejects something
+// that the evaluator accepts).
+//
+// Using `XFAIL ... *` with `not` on the expected-to-pass lines isn't appropriate,
+// it seems, because that will cause the test to pass when _any_ of the RUNs
+// fail.
+//
+// We could use a RUN that groups all four commands into a single shell
+// invocation that expresses the desired logical properties, possibly negating
+// and using an `XFAIL` for clarity (?), but I suspect the long-term future
+// of this test file is to get out of this situation and back into the "both
+// match" category anyway.
+//
+// RUN: %clang_cc1 -verify=ref,both -std=c99 %s
+// RUN: %clang_cc1 -verify=ref,both -std=c11 %s
+// RUN: %clang_cc1 -verify=ref,both -std=c2x %s
+//
+// RUN: %clang_cc1 -verify=expected,both -std=c99 -fexperimental-new-constant-interpreter %s
+// RUN: %clang_cc1 -verify=expected,both -std=c11 -fexperimental-new-constant-interpreter %s
+// RUN: %clang_cc1 -verify=expected,both -std=c2x -fexperimental-new-constant-interpreter %s
+
+#pragma clang diagnostic ignored "-Wgnu-folding-constant"
+#pragma clang diagnostic ignored "-Wempty-translation-unit"
+
+#if __STDC_VERSION__ >= 201112L
+#define CHECK(cond) _Static_assert((cond), "")
+#else
+#pragma clang diagnostic ignored "-Wextra-semi"
+#define CHECK(cond)
+#endif
+
+typedef struct {
+ unsigned a, b;
+ char cc[2];
+} s_t;
+
+// out-of-order designated initialization
+// array designated initialization
+const s_t s1 = { .a = 2, .b = 4, .cc[0] = 8, .cc[1] = 16 } ;
+const s_t s2 = { .b = 4, .a = 2, .cc[1] = 16, .cc[0] = 8 } ;
+
+CHECK(s1.a == s2.a && s1.b == s2.b);
+CHECK(s1.cc[0] == s2.cc[0] && s1.cc[1] == s2.cc[1]);
+
+// nested designated initialization
+const struct {
+ struct { unsigned v; } inner;
+} nested_designated = { .inner.v = 3 };
+CHECK(nested_designated.inner.v == 3);
+
+// mixing of designated initializers and regular initializers
+// both-warning@+1 {{excess elements in array initializer}}
+const s_t s3 = { {}, .b = 4, {[1]=16, 8}};
+const s_t s4 = { .b = 4, {[1]=16}};
+
+CHECK(s3.a == 0);
+CHECK(s3.b == 4);
+CHECK(s3.cc[0] == 0);
+CHECK(s3.cc[1] == 16);
+
+CHECK(s3.a == s4.a && s3.b == s4.b);
+CHECK(s3.cc[0] == s4.cc[0] && s3.cc[1] == s4.cc[1]);
+
+const unsigned fw = 2;
+typedef struct {
+ struct {
+ unsigned : 4;
+ unsigned ff : fw;
+ unsigned : 12;
+ unsigned : 12;
+ } in[2];
+
+ unsigned of : 4;
+ unsigned : 0;
+} bf_t;
+
+const bf_t bf0 = { };
+CHECK(bf0.in[0].ff == 0);
+CHECK(bf0.in[1].ff == 0);
+CHECK(bf0.of == 0);
+
+CHECK(((bf_t){{{}, {}}, {}}).of == 0);
+CHECK(((bf_t){{{}, {}}, {}}).of == 0);
+CHECK(((bf_t){{{}, {1}}, {}}).in[1].ff == 1);
+
+// out-of-order designated initialization
+// array designated initialization
+// nested designated initialization
+// mixing of designated initializers and regular initializers
+// + skipped fields (unnamed bit fields)
+const bf_t bf1 = { 1, 2, 3, };
+const bf_t bf2 = { { [1]=2, [0]={ 1 }}, 3, };
+
+CHECK(bf1.in[0].ff == 1);
+CHECK(bf1.in[1].ff == 2);
+CHECK(bf1.of == 3);
+
+CHECK(
+ bf1.in[0].ff == bf2.in[0].ff &&
+ bf1.in[1].ff == bf2.in[1].ff &&
+ bf1.of == bf2.of
+);
+
+unsigned func() {
+ return s1.a + s2.b + bf1.of + bf2.of;
+}
diff --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index 0f76e0cfe99277..89ac51620fc4cc 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -415,7 +415,7 @@ namespace DeriveFailures {
constexpr Derived(int i) : OtherVal(i) {} // ref-error {{never produces a constant expression}} \
// both-note {{non-constexpr constructor 'Base' cannot be used in a constant expression}} \
- // ref-note {{non-constexpr constructor 'Base' cannot be used in a constant expression}}
+ // ref-note {{non-constexpr constructor 'Base' cannot be used in a constant expression}}
};
constexpr Derived D(12); // both-error {{must be initialized by a constant expression}} \
@@ -1285,3 +1285,92 @@ namespace {
}
}
#endif
+
+#if __cplusplus >= 201703L
+namespace InitLists {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wmissing-braces"
+#pragma clang diagnostic ignored "-Wc99-designator"
+#pragma clang diagnostic ignored "-Wc++20-extensions"
+
+ struct EmptyBase {};
+
+ struct A { char x; };
+ struct B { short y; unsigned z; };
+
+ struct S : A, B, EmptyBase {
+ constexpr bool operator==(const S& rhs) const {
+ return this->x == rhs.x && this->y == rhs.y && this->z == rhs.z;
+ };
+ };
+
+ constexpr S s0 = {};
+ static_assert(s0.x == 0 && s0.y == 0 && s0.z == 0);
+
+ static_assert(S{1} == S{A{1}, B{0, 0}}, "");
+ static_assert(S{1, 2} == S{A{1}, B{2, 0}}, "");
+ static_assert(S{1, 2, 3} == S{A{1}, B{2, 3}}, "");
+
+ static_assert(S{1, {2, 3}} == S{A{1}, B{2, 3}}, "");
+ static_assert(S{{1}, {2, 3}} == S{A{1}, B{2, 3}}, "");
+
+ struct BF : S {
+ unsigned : 12;
+ unsigned f1 : 3;
+ unsigned : 34;
+ unsigned : 34;
+ unsigned f2 : 3;
+ unsigned : 0;
+
+ bool operator==(const BF&) const = default;
+ };
+
+ static_assert(BF{} == BF{{s0}, 0, 0}, "");
+ static_assert(BF{1, 2, 3} == BF{S{A{1}, B{2, 3}, {}}, 0, 0}, "");
+
+ static_assert(BF{{}, 4} == BF{s0, .f1 = 4});
+ static_assert(BF{{}, 4, 5} == BF{s0, .f1 = 4, .f2 = 5});
+ static_assert(BF{{1, 2, 3}, 4} == BF{S{A{1}, B{2, 3}, {}}, .f1 = 4}, "");
+ static_assert(BF{1, 2, 3, {}, 4} == BF{S{A{1}, B{2, 3}, {}}, .f1 = 4}, "");
+ static_assert(BF{1, 2, 3, {}, 4, 5}
+ == BF{S{A{1}, B{2, 3}, {}}, .f1 = 4, .f2 = 5}, "");
+
+
+ struct R : BF {
+ unsigned ff : 3;
+ struct {
+ char cc[2];
+ } rr[2];
+
+ constexpr bool operator==(const R& rhs) const {
+ return static_cast<BF>(*this) == static_cast<BF>(rhs)
+ && this->ff == rhs.ff
+ && this->rr[0].cc[0] == rhs.rr[0].cc[0]
+ && this->rr[0].cc[1] == rhs.rr[0].cc[1]
+ && this->rr[1].cc[0] == rhs.rr[1].cc[0]
+ && this->rr[1].cc[1] == rhs.rr[1].cc[1]
+ ;
+ };
+ };
+
+ static_assert(R{} == R{{s0}, 0}, "");
+ static_assert(R{{}, 6} == R{{s0}, .ff = 6, .rr = {}}, "");
+ static_assert(R{{}, 6, 7} ==
+ R{{s0}, .ff = 6, .rr = {[0]={.cc = {[0]=7, [1]=0}}, [1]={.cc = {[0]=0, [1]=0}}}}, "");
+ static_assert(R{{}, 6, 7, 8} == R{{s0}, 6, {{7, 8}}}, "");
+
+ constexpr R r = {{{1, 2, 3}, 4, 5}, 6, 7, 8, 9, 10};
+ static_assert(r.x == 1, "");
+ static_assert(r.y == 2, "");
+ static_assert(r.z == 3, "");
+ static_assert(r.f1 == 4, "");
+ static_assert(r.f2 == 5, "");
+ static_assert(r.ff == 6, "");
+ static_assert(r.rr[0].cc[0] == 7, "");
+ static_assert(r.rr[0].cc[1] == 8, "");
+ static_assert(r.rr[1].cc[0] == 9, "");
+ static_assert(r.rr[1].cc[1] == 10, "");
+
+#pragma clang diagnostic pop
+} // namespace InitLists
+#endif
|
sethp
commented
Apr 5, 2024
|
|
||
| const bf_t bf0 = { }; | ||
| CHECK(bf0.in[0].ff == 0); | ||
| CHECK(bf0.in[1].ff == 0); |
There was a problem hiding this comment.
This is the line that crashes: somehow it makes it to
with a non-primitive element type.backtrace
not a primitive type
UNREACHABLE executed at /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.h:156!
Stack dump:
0. Program arguments: /home/seth/Code/src/github.com/llvm/llvm-project/build/bin/clang -cc1 -internal-isystem /home/seth/Code/src/github.com/llvm/llvm-project/build/lib/clang/18/include -nostdsysteminc -verify -std=c2x -fsyntax-only -fexperimental-new-constant-interpreter -triple=x86_64-apple-macosx10.14.0 /home/seth/Code/src/github.com/llvm/llvm-project/clang/test/AST/Interp/records.c
1. /home/seth/Code/src/github.com/llvm/llvm-project/clang/test/AST/Interp/records.c:87:1 <Spelling=/home/seth/Code/src/github.com/llvm/llvm-project/clang/test/AST/Interp/records.c:34:21>: current parser token '_Static_assert'
#0 0x0000555564304b66 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/seth/Code/src/github.com/llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:723:22
#1 0x0000555564304f79 PrintStackTraceSignalHandler(void*) /home/seth/Code/src/github.com/llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:798:1
#2 0x00005555643023b1 llvm::sys::RunSignalHandlers() /home/seth/Code/src/github.com/llvm/llvm-project/llvm/lib/Support/Signals.cpp:105:20
#3 0x000055556430444d SignalHandler(int) /home/seth/Code/src/github.com/llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:413:1
#4 0x00007ffff785a770 (/usr/lib/libc.so.6+0x40770)
#5 0x00007ffff78ab32c (/usr/lib/libc.so.6+0x9132c)
#6 0x00007ffff785a6c8 raise (/usr/lib/libc.so.6+0x406c8)
#7 0x00007ffff78424b8 abort (/usr/lib/libc.so.6+0x284b8)
#8 0x0000555564224207 bindingsErrorHandler(void*, char const*, bool) /home/seth/Code/src/github.com/llvm/llvm-project/llvm/lib/Support/ErrorHandling.cpp:222:55
#9 0x000055556a88586f clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::classifyPrim(clang::QualType) const /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.h:157:3
#10 0x000055556a87f407 clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::VisitImplicitValueInitExpr(clang::ImplicitValueInitExpr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:823:34
#11 0x000055556a89004e clang::StmtVisitorBase<llvm::make_const_ptr, clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>, bool>::Visit(clang::Stmt const*) /home/seth/Code/src/github.com/llvm/llvm-project/build/tools/clang/include/clang/AST/StmtNodes.inc:364:1
#12 0x000055556a885b7c clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visitInitializer(clang::Expr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:2321:21
#13 0x000055556a886d81 clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visitInitList(llvm::ArrayRef<clang::Expr const*>, clang::Expr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:944:34
#14 0x000055556a87f9d9 clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::VisitInitListExpr(clang::InitListExpr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:999:45
#15 0x000055556a890036 clang::StmtVisitorBase<llvm::make_const_ptr, clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>, bool>::Visit(clang::Stmt const*) /home/seth/Code/src/github.com/llvm/llvm-project/build/tools/clang/include/clang/AST/StmtNodes.inc:358:1
#16 0x000055556a885b7c clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visitInitializer(clang::Expr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:2321:21
#17 0x000055556a886594 clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visitGlobalInitializer(clang::Expr const*, unsigned int) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.h:200:26
#18 0x000055556a885ebd clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visitVarDecl(clang::VarDecl const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:2724:42
#19 0x000055556a87f1af clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::VisitDeclRefExpr(clang::DeclRefExpr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:3336:30
#20 0x000055556a8901b6 clang::StmtVisitorBase<llvm::make_const_ptr, clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>, bool>::Visit(clang::Stmt const*) /home/seth/Code/src/github.com/llvm/llvm-project/build/tools/clang/include/clang/AST/StmtNodes.inc:462:1
#21 0x000055556a885a6d clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visit(clang::Expr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:2309:21
#22 0x000055556a880443 clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::VisitMemberExpr(clang::MemberExpr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:1196:21
#23 0x000055556a88ff8e clang::StmtVisitorBase<llvm::make_const_ptr, clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>, bool>::Visit(clang::Stmt const*) /home/seth/Code/src/github.com/llvm/llvm-project/build/tools/clang/include/clang/AST/StmtNodes.inc:316:1
#24 0x000055556a885a6d clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visit(clang::Expr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:2309:21
#25 0x000055556a878e0e clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::VisitCastExpr(clang::CastExpr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:193:21
#26 0x000055556a896b79 clang::StmtVisitorBase<llvm::make_const_ptr, clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>, bool>::VisitImplicitCastExpr(clang::ImplicitCastExpr const*) /home/seth/Code/src/github.com/llvm/llvm-project/build/tools/clang/include/clang/AST/StmtNodes.inc:522:1
#27 0x000055556a890276 clang::StmtVisitorBase<llvm::make_const_ptr, clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>, bool>::Visit(clang::Stmt const*) /home/seth/Code/src/github.com/llvm/llvm-project/build/tools/clang/include/clang/AST/StmtNodes.inc:522:1
#28 0x000055556a885a6d clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visit(clang::Expr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:2309:21
#29 0x000055556a87f6a6 clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::VisitArraySubscriptExpr(clang::ArraySubscriptExpr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:862:19
#30 0x000055556a890696 clang::StmtVisitorBase<llvm::make_const_ptr, clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>, bool>::Visit(clang::Stmt const*) /home/seth/Code/src/github.com/llvm/llvm-project/build/tools/clang/include/clang/AST/StmtNodes.inc:812:1
#31 0x000055556a885a6d clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visit(clang::Expr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:2309:21
#32 0x000055556a880443 clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::VisitMemberExpr(clang::MemberExpr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:1196:21
#33 0x000055556a88ff8e clang::StmtVisitorBase<llvm::make_const_ptr, clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>, bool>::Visit(clang::Stmt const*) /home/seth/Code/src/github.com/llvm/llvm-project/build/tools/clang/include/clang/AST/StmtNodes.inc:316:1
#34 0x000055556a885a6d clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visit(clang::Expr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:2309:21
#35 0x000055556a88504a clang::interp::ByteCodeExprGen<clang::interp::EvalEmitter>::visitExpr(clang::Expr const*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/ByteCodeExprGen.cpp:2622:15
#36 0x000055556a8c5740 clang::interp::EvalEmitter::interpretExpr(clang::Expr const*, bool) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/EvalEmitter.cpp:40:7
#37 0x000055556a85e2b7 clang::interp::Context::evaluateAsRValue(clang::interp::State&, clang::Expr const*, clang::APValue&) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/Interp/Context.cpp:48:20
#38 0x000055556a7cae65 EvaluateAsRValue((anonymous namespace)::EvalInfo&, clang::Expr const*, clang::APValue&) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/ExprConstant.cpp:15590:9
#39 0x000055556a7cb416 EvaluateAsRValue(clang::Expr const*, clang::Expr::EvalResult&, clang::ASTContext const&, (anonymous namespace)::EvalInfo&) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/ExprConstant.cpp:15664:46
#40 0x000055556a7cb788 clang::Expr::EvaluateAsRValue(clang::Expr::EvalResult&, clang::ASTContext const&, bool) const /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/AST/ExprConstant.cpp:15713:28
#41 0x0000555568b852e5 GetExprRange(clang::ASTContext&, clang::Expr const*, unsigned int, bool, bool) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaChecking.cpp:14676:3
#42 0x0000555568b8612d GetExprRange(clang::ASTContext&, clang::Expr const*, bool, bool) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaChecking.cpp:14922:1
#43 0x0000555568b86ec5 CheckTautologicalComparison(clang::Sema&, clang::BinaryOperator*, clang::Expr*, clang::Expr*, llvm::APSInt const&, bool) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaChecking.cpp:15166:42
#44 0x0000555568b87dc0 AnalyzeComparison(clang::Sema&, clang::BinaryOperator*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaChecking.cpp:15341:7
#45 0x0000555568b8f0d6 AnalyzeImplicitConversions(clang::Sema&, (anonymous namespace)::AnalyzeImplicitConversionsWorkItem, llvm::SmallVectorImpl<(anonymous namespace)::AnalyzeImplicitConversionsWorkItem>&) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaChecking.cpp:16709:37
#46 0x0000555568b8f6c1 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaChecking.cpp:16780:25
#47 0x0000555568b90cce clang::Sema::CheckImplicitConversions(clang::Expr*, clang::SourceLocation) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaChecking.cpp:17074:29
#48 0x0000555568b943a4 clang::Sema::CheckCompletedExpr(clang::Expr*, clang::SourceLocation, bool) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaChecking.cpp:18007:35
#49 0x000055556951ef82 clang::Sema::ActOnFinishFullExpr(clang::Expr*, clang::SourceLocation, bool, bool, bool) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaExprCXX.cpp:9029:19
#50 0x000055556905b0b6 clang::Sema::BuildStaticAssertDeclaration(clang::SourceLocation, clang::Expr*, clang::Expr*, clang::SourceLocation, bool) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaDeclCXX.cpp:17468:28
#51 0x00005555690592df clang::Sema::ActOnStaticAssertDeclaration(clang::SourceLocation, clang::Expr*, clang::Expr*, clang::SourceLocation) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Sema/SemaDeclCXX.cpp:17112:74
#52 0x000055556885bce7 clang::Parser::ParseStaticAssertDeclaration(clang::SourceLocation&) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Parse/ParseDeclCXX.cpp:1063:46
#53 0x0000555568827e7e clang::Parser::ParseDeclaration(clang::DeclaratorContext, clang::SourceLocation&, clang::ParsedAttributes&, clang::ParsedAttributes&, clang::SourceLocation*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Parse/ParseDecl.cpp:1956:46
#54 0x000055556880b14e clang::Parser::ParseExternalDeclaration(clang::ParsedAttributes&, clang::ParsedAttributes&, clang::ParsingDeclSpec*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Parse/Parser.cpp:988:30
#55 0x000055556880a48e clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, clang::Sema::ModuleImportState&) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Parse/Parser.cpp:762:36
#56 0x00005555688056d3 clang::ParseAST(clang::Sema&, bool, bool) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Parse/ParseAST.cpp:163:37
#57 0x00005555657be248 clang::ASTFrontendAction::ExecuteAction() /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Frontend/FrontendAction.cpp:1189:11
#58 0x00005555657bdb36 clang::FrontendAction::Execute() /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Frontend/FrontendAction.cpp:1079:38
#59 0x00005555656d7c6f clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1062:42
#60 0x000055556597227e clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:274:38
#61 0x000055556031445a cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /home/seth/Code/src/github.com/llvm/llvm-project/clang/tools/driver/cc1_main.cpp:232:40
#62 0x0000555560305e79 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) /home/seth/Code/src/github.com/llvm/llvm-project/clang/tools/driver/driver.cpp:215:20
#63 0x00005555603063c7 clang_main(int, char**, llvm::ToolContext const&) /home/seth/Code/src/github.com/llvm/llvm-project/clang/tools/driver/driver.cpp:256:26
#64 0x000055556033e66c main /home/seth/Code/src/github.com/llvm/llvm-project/build/tools/clang/tools/driver/clang-driver.cpp:17:20
#65 0x00007ffff7843cd0 (/usr/lib/libc.so.6+0x29cd0)
#66 0x00007ffff7843d8a __libc_start_main (/usr/lib/libc.so.6+0x29d8a)
#67 0x0000555560305225 _start (/home/seth/Code/src/github.com/llvm/llvm-project/build/bin/clang+0xadb1225)
It might also affect C++-land, come to think of it, since I think Sema has a different Expr for {} than T{}, right?
tbaederr
reviewed
Apr 6, 2024
| @@ -0,0 +1,114 @@ | |||
| // UNSUPPORTED: asserts | |||
| // REQUIRES: asserts | |||
| // ^ this attempts to say "don't actually run this test", because it's broken | |||
There was a problem hiding this comment.
It's broken even with the changes in this PR?
There was a problem hiding this comment.
Yeah, the test was meant to validate the "iterating the init expressions in the init list in-order will work for C records", but it doesn't get that far: in trying to do so, I uncovered a different bug.
What do you usually do, when that happens? Stop and dig out the bug? Or do you have some way you're deferring/keeping track of those kinds of things?
There was a problem hiding this comment.
In this case, I would fix the other bug first.
There was a problem hiding this comment.
Sounds good, I'll take a look at it when I get a chance.
| // ^ this attempts to say "don't actually run this test", because it's broken | ||
| // | ||
| // The point of this test is to demonstrate something that ExprConstant accepts, | ||
| // but Interp rejects. I had hoped to express that as the same file with two |
There was a problem hiding this comment.
Please stop calling it "Interp", it just sounds silly.
There was a problem hiding this comment.
😅 sorry, I got that from the abbreviation in the commit messages.
What do you call it, when you need to distinguish it from the recursive-descent implementation?
There was a problem hiding this comment.
"The new constant interpreter" or a variation thereof.
| @@ -884,8 +884,36 @@ bool ByteCodeExprGen<Emitter>::visitInitList(ArrayRef<const Expr *> Inits, | |||
| if (!this->emitDupPtr(E)) | |||
| return false; | |||
|
|
|||
| // guard relatively expensive base check behind an almost-always-false | |||
There was a problem hiding this comment.
The comment sounds like this is just a performance improvement. Is that so?
There was a problem hiding this comment.
Maybe half so? It's an attempt to avoid a performance regression that I think I would've otherwise introduced by making this change.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, types like this one would be mis-initialized:
Because, although the unnamed bit-field has an index in the Record, it
does not actually take any initializer values from an expression such
as the above.
This one's for @tbaederr: it's broken out from the work I was doing experimenting with the bit-field bit-casting tests, since it seems mostly self-contained.
While separating it out, I had the question of "will iterating the Inits in order always work?": it will for C++, I think, but I'm not so sure about C. I wrote a test to find out (63587ae), and I manually-fuzzed my way into a failure for an unrelated reason that seems C-specific (a similar construct in
records.cppworked just fine).I was curious what you'd like to do: hold here awaiting a fix for the underlying issue? Pull the failing
records.cinto a different change? Merge the failing test as a future pending task?Thanks!