[clang][bytecode] Change the way we do init chains #122871
Merged
Conversation
This file contains hidden or 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
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesSee the comment in Compiler<>::VisitCXXThisExpr. Full diff: https://github.com/llvm/llvm-project/pull/122871.diff 4 Files Affected:
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 036f9608bf3ca1..b97d54ece4e598 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -90,6 +90,8 @@ bool InitLink::emit(Compiler<Emitter> *Ctx, const Expr *E) const {
if (!Ctx->emitConstUint32(Offset, E))
return false;
return Ctx->emitArrayElemPtrPopUint32(E);
+ case K_InitList:
+ return true;
default:
llvm_unreachable("Unhandled InitLink kind");
}
@@ -1717,6 +1719,8 @@ bool Compiler<Emitter>::VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
template <class Emitter>
bool Compiler<Emitter>::visitInitList(ArrayRef<const Expr *> Inits,
const Expr *ArrayFiller, const Expr *E) {
+ InitLinkScope<Emitter> ILS(this, InitLink::InitList());
+
QualType QT = E->getType();
if (const auto *AT = QT->getAs<AtomicType>())
QT = AT->getValueType();
@@ -1754,6 +1758,7 @@ bool Compiler<Emitter>::visitInitList(ArrayRef<const Expr *> Inits,
auto initPrimitiveField = [=](const Record::Field *FieldToInit,
const Expr *Init, PrimType T) -> bool {
InitStackScope<Emitter> ISS(this, isa<CXXDefaultInitExpr>(Init));
+ InitLinkScope<Emitter> ILS(this, InitLink::Field(FieldToInit->Offset));
if (!this->visit(Init))
return false;
@@ -1766,6 +1771,7 @@ bool Compiler<Emitter>::visitInitList(ArrayRef<const Expr *> Inits,
const Expr *Init) -> bool {
InitStackScope<Emitter> ISS(this, isa<CXXDefaultInitExpr>(Init));
InitLinkScope<Emitter> ILS(this, InitLink::Field(FieldToInit->Offset));
+
// Non-primitive case. Get a pointer to the field-to-initialize
// on the stack and recurse into visitInitializer().
if (!this->emitGetPtrField(FieldToInit->Offset, Init))
@@ -3812,6 +3818,7 @@ template <class Emitter> bool Compiler<Emitter>::visit(const Expr *E) {
if (!this->emitGetPtrLocal(*LocalIndex, E))
return false;
+ InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex));
return this->visitInitializer(E);
}
@@ -4848,18 +4855,49 @@ bool Compiler<Emitter>::VisitCXXThisExpr(const CXXThisExpr *E) {
// instance pointer of the current function frame, but e.g. to the declaration
// currently being initialized. Here we emit the necessary instruction(s) for
// this scenario.
- if (!InitStackActive || !E->isImplicit())
+ if (!InitStackActive)
return this->emitThis(E);
- if (InitStackActive && !InitStack.empty()) {
+ if (!InitStack.empty()) {
+ // If our init stack is, for example:
+ // 0 Stack: 3 (decl)
+ // 1 Stack: 6 (init list)
+ // 2 Stack: 1 (field)
+ // 3 Stack: 6 (init list)
+ // 4 Stack: 1 (field)
+ //
+ // We want to find the LAST element in it that's an init list,
+ // which is marked with the K_InitList marker. The index right
+ // before that points to an init list. We need to find the
+ // elements before the K_InitList element that point to a base
+ // (e.g. a decl or This), optionally followed by field, elem, etc.
+ // In the example above, we want to emit elements [0..2].
unsigned StartIndex = 0;
+ unsigned EndIndex = 0;
+ // Findteh init list.
for (StartIndex = InitStack.size() - 1; StartIndex > 0; --StartIndex) {
+ if (InitStack[StartIndex].Kind == InitLink::K_InitList ||
+ InitStack[StartIndex].Kind == InitLink::K_This) {
+ EndIndex = StartIndex;
+ --StartIndex;
+ break;
+ }
+ }
+
+ // Walk backwards to find the base.
+ for (; StartIndex > 0; --StartIndex) {
+ if (InitStack[StartIndex].Kind == InitLink::K_InitList)
+ continue;
+
if (InitStack[StartIndex].Kind != InitLink::K_Field &&
InitStack[StartIndex].Kind != InitLink::K_Elem)
break;
}
- for (unsigned I = StartIndex, N = InitStack.size(); I != N; ++I) {
+ // Emit the instructions.
+ for (unsigned I = StartIndex; I != EndIndex; ++I) {
+ if (InitStack[I].Kind == InitLink::K_InitList)
+ continue;
if (!InitStack[I].template emit<Emitter>(this, E))
return false;
}
diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h
index 71765b18cb1a90..2d5b76f789543e 100644
--- a/clang/lib/AST/ByteCode/Compiler.h
+++ b/clang/lib/AST/ByteCode/Compiler.h
@@ -51,9 +51,11 @@ struct InitLink {
K_Temp = 2,
K_Decl = 3,
K_Elem = 5,
+ K_InitList = 6
};
static InitLink This() { return InitLink{K_This}; }
+ static InitLink InitList() { return InitLink{K_InitList}; }
static InitLink Field(unsigned Offset) {
InitLink IL{K_Field};
IL.Offset = Offset;
diff --git a/clang/test/AST/ByteCode/records.cpp b/clang/test/AST/ByteCode/records.cpp
index 4601aface135ee..d329219264d893 100644
--- a/clang/test/AST/ByteCode/records.cpp
+++ b/clang/test/AST/ByteCode/records.cpp
@@ -1678,3 +1678,9 @@ namespace NonConst {
static_assert(s.getSize() == 10, "");
}
}
+
+namespace ExplicitThisInTemporary {
+ struct B { B *p = this; };
+ constexpr bool g(B b) { return &b == b.p; }
+ static_assert(g({}), "");
+}
diff --git a/clang/test/AST/ByteCode/unions.cpp b/clang/test/AST/ByteCode/unions.cpp
index 7b39bb1bb9316e..e90b123c90de0e 100644
--- a/clang/test/AST/ByteCode/unions.cpp
+++ b/clang/test/AST/ByteCode/unions.cpp
@@ -401,4 +401,24 @@ namespace UnionInBase {
// both-note {{subobject 'y' is not initialized}}
static_assert(return_uninit().a.x == 2);
}
+
+/// FIXME: Our diagnostic here is a little off.
+namespace One {
+ struct A { long x; };
+
+ union U;
+ constexpr A foo(U *up);
+ union U {
+ A a = foo(this); // both-note {{in call to 'foo(&u)'}}
+ int y;
+ };
+
+ constexpr A foo(U *up) {
+ return {up->y}; // both-note {{read of member 'y' of union}}
+ }
+
+ constinit U u = {}; // both-error {{constant init}} \
+ // both-note {{constinit}}
+}
+
#endif
|
See the comment in Compiler<>::VisitCXXThisExpr. We need to mark the InitList explicitly, so we later know what to refer to when the init chain is active.
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/12782 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
See the comment in Compiler<>::VisitCXXThisExpr.
We need to mark the InitList explicitly, so we later know what to refer to when the init chain is active.