-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang][bytecode] Loosen assertion This() for array elements #130399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
getRecord() returns null on array elements, even for composite arrays. The assertion here was overly restrictive and having an array element as instance pointer should be fine otherwise.
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesgetRecord() returns null on array elements, even for composite arrays. The assertion here was overly restrictive and having an array element as instance pointer should be fine otherwise. Full diff: https://github.com/llvm/llvm-project/pull/130399.diff 2 Files Affected:
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index d8f90e45b0ced..8a36db1cd84b4 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2431,9 +2431,12 @@ inline bool This(InterpState &S, CodePtr OpPC) {
// Ensure the This pointer has been cast to the correct base.
if (!This.isDummy()) {
assert(isa<CXXMethodDecl>(S.Current->getFunction()->getDecl()));
- assert(This.getRecord());
+ [[maybe_unused]] const Record *R = This.getRecord();
+ if (!R)
+ R = This.narrow().getRecord();
+ assert(R);
assert(
- This.getRecord()->getDecl() ==
+ R->getDecl() ==
cast<CXXMethodDecl>(S.Current->getFunction()->getDecl())->getParent());
}
diff --git a/clang/test/AST/ByteCode/placement-new.cpp b/clang/test/AST/ByteCode/placement-new.cpp
index 7a4fc89a27dac..c353162a7aab0 100644
--- a/clang/test/AST/ByteCode/placement-new.cpp
+++ b/clang/test/AST/ByteCode/placement-new.cpp
@@ -339,6 +339,30 @@ namespace PR48606 {
static_assert(f());
}
+/// This used to crash because of an assertion in the implementation
+/// of the This instruction.
+namespace ExplicitThisOnArrayElement {
+ struct S {
+ int a = 12;
+ constexpr S(int a) {
+ this->a = a;
+ }
+ };
+
+ template <class _Tp, class... _Args>
+ constexpr void construct_at(_Tp *__location, _Args &&...__args) {
+ new (__location) _Tp(__args...);
+ }
+
+ constexpr bool foo() {
+ auto *M = std::allocator<S>().allocate(13); // both-note {{allocation performed here was not deallocated}}
+ construct_at(M, 12);
+ return true;
+ }
+
+ static_assert(foo()); // both-error {{not an integral constant expression}}
+}
+
#ifdef BYTECODE
constexpr int N = [] // expected-error {{must be initialized by a constant expression}} \
// expected-note {{assignment to dereferenced one-past-the-end pointer is not allowed in a constant expression}} \
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/13224 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/94/builds/5061 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/6640 Here is the relevant piece of the build log for the reference
|
getRecord() returns null on array elements, even for composite arrays. The assertion here was overly restrictive and having an array element as instance pointer should be fine otherwise.