[PATCH] D144016: [Sema] Relax a failing assertion in TransformBlockExpr
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcda4a0e918b5: [Sema] Relax a failing assertion in
TransformBlockExpr (authored by ahatanak).
Changed prior to commit:
https://reviews.llvm.org/D144016?vs=497776=498232#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144016/new/
https://reviews.llvm.org/D144016
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/TreeTransform.h
clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
Index: clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s
--implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -fblocks -triple
x86_64-apple-darwin10 -o - | FileCheck %s
--implicit-check-not=should_not_be_used
void should_be_used_1();
void should_be_used_2();
@@ -32,3 +32,20 @@
// CHECK: should_be_used_1
// CHECK: should_be_used_2
// CHECK: should_be_used_3
+
+namespace BlockThisCapture {
+ void foo();
+ struct S {
+template
+void m() {
+ ^{ if constexpr(b) (void)this; else foo(); }();
+}
+ };
+
+ void test() {
+S().m();
+ }
+}
+
+// CHECK-LABEL: define internal void
@___ZN16BlockThisCapture1S1mILb0EEEvv_block_invoke(
+// CHECK: call void @_ZN16BlockThisCapture3fooEv(
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -14598,7 +14598,12 @@
oldCapture));
assert(blockScope->CaptureMap.count(newCapture));
}
-assert(oldBlock->capturesCXXThis() == blockScope->isCXXThisCaptured());
+
+// The this pointer may not be captured by the instantiated block, even
when
+// it's captured by the original block, if the expression causing the
+// capture is in the discarded branch of a constexpr if statement.
+assert((!blockScope->isCXXThisCaptured() || oldBlock->capturesCXXThis()) &&
+ "this pointer isn't captured in the old block");
}
#endif
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -141,6 +141,9 @@
driver mode and emit an error which suggests using ``/TC`` or ``/TP``
``clang-cl`` options instead.
(`#59307 `_)
+- Fix assert that fails when the expression causing the this pointer to be
+ captured by a block is part of a constexpr if statement's branch and
+ instantiation of the enclosing method causes the branch to be discarded.
Bug Fixes to Compiler Builtins
^^
Index: clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s --implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -fblocks -triple x86_64-apple-darwin10 -o - | FileCheck %s --implicit-check-not=should_not_be_used
void should_be_used_1();
void should_be_used_2();
@@ -32,3 +32,20 @@
// CHECK: should_be_used_1
// CHECK: should_be_used_2
// CHECK: should_be_used_3
+
+namespace BlockThisCapture {
+ void foo();
+ struct S {
+template
+void m() {
+ ^{ if constexpr(b) (void)this; else foo(); }();
+}
+ };
+
+ void test() {
+S().m();
+ }
+}
+
+// CHECK-LABEL: define internal void @___ZN16BlockThisCapture1S1mILb0EEEvv_block_invoke(
+// CHECK: call void @_ZN16BlockThisCapture3fooEv(
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -14598,7 +14598,12 @@
oldCapture));
assert(blockScope->CaptureMap.count(newCapture));
}
-assert(oldBlock->capturesCXXThis() == blockScope->isCXXThisCaptured());
+
+// The this pointer may not be captured by the instantiated block, even when
+// it's captured by the original block, if the expression causing the
+// capture is in the discarded branch of a constexpr if statement.
+assert((!blockScope->isCXXThisCaptured() || oldBlock->capturesCXXThis()) &&
+ "this pointer isn't captured in the old block");
}
#endif
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -141,6 +141,9 @@
driver
[PATCH] D144016: [Sema] Relax a failing assertion in TransformBlockExpr
ahatanak updated this revision to Diff 497776.
ahatanak added a comment.
Add triple to the test and add release note for the fix.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144016/new/
https://reviews.llvm.org/D144016
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/TreeTransform.h
clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
Index: clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s
--implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -fblocks -triple
x86_64-apple-darwin10 -o - | FileCheck %s
--implicit-check-not=should_not_be_used
void should_be_used_1();
void should_be_used_2();
@@ -32,3 +32,20 @@
// CHECK: should_be_used_1
// CHECK: should_be_used_2
// CHECK: should_be_used_3
+
+namespace BlockThisCapture {
+ void foo();
+ struct S {
+template
+void m() {
+ ^{ if constexpr(b) (void)this; else foo(); }();
+}
+ };
+
+ void test() {
+S().m();
+ }
+}
+
+// CHECK-LABEL: define internal void
@___ZN16BlockThisCapture1S1mILb0EEEvv_block_invoke(
+// CHECK: call void @_ZN16BlockThisCapture3fooEv(
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -14598,7 +14598,12 @@
oldCapture));
assert(blockScope->CaptureMap.count(newCapture));
}
-assert(oldBlock->capturesCXXThis() == blockScope->isCXXThisCaptured());
+
+// The this pointer may not be captured by the instantiated block, even
when
+// it's captured by the original block, if the expression causing the
+// capture is in the discarded branch of a constexpr if statement.
+assert((!blockScope->isCXXThisCaptured() || oldBlock->capturesCXXThis()) &&
+ "this pointer isn't captured in the old block");
}
#endif
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -67,6 +67,9 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field. This fixes
`Issue 60166 `_.
+- Fix assert that fails when the expression causing the this pointer to be
+ captured by a block is part of a constexpr if statement's branch and
+ instantiation of the enclosing method causes the branch to be discarded.
Improvements to Clang's diagnostics
^^^
Index: clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s --implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -fblocks -triple x86_64-apple-darwin10 -o - | FileCheck %s --implicit-check-not=should_not_be_used
void should_be_used_1();
void should_be_used_2();
@@ -32,3 +32,20 @@
// CHECK: should_be_used_1
// CHECK: should_be_used_2
// CHECK: should_be_used_3
+
+namespace BlockThisCapture {
+ void foo();
+ struct S {
+template
+void m() {
+ ^{ if constexpr(b) (void)this; else foo(); }();
+}
+ };
+
+ void test() {
+S().m();
+ }
+}
+
+// CHECK-LABEL: define internal void @___ZN16BlockThisCapture1S1mILb0EEEvv_block_invoke(
+// CHECK: call void @_ZN16BlockThisCapture3fooEv(
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -14598,7 +14598,12 @@
oldCapture));
assert(blockScope->CaptureMap.count(newCapture));
}
-assert(oldBlock->capturesCXXThis() == blockScope->isCXXThisCaptured());
+
+// The this pointer may not be captured by the instantiated block, even when
+// it's captured by the original block, if the expression causing the
+// capture is in the discarded branch of a constexpr if statement.
+assert((!blockScope->isCXXThisCaptured() || oldBlock->capturesCXXThis()) &&
+ "this pointer isn't captured in the old block");
}
#endif
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -67,6 +67,9 @@
- Fix crash when evaluating consteval constructor of derived class whose base
has more than one field. This fixes
`Issue 60166
[PATCH] D144016: [Sema] Relax a failing assertion in TransformBlockExpr
aaron.ballman added a comment.
Precommit CI found an issue on Windows that should be addressed:
FAIL: Clang :: CodeGenCXX/cxx1z-constexpr-if.cpp (7014 of 17903)
TEST 'Clang :: CodeGenCXX/cxx1z-constexpr-if.cpp' FAILED
Script:
--
: 'RUN: at line 1';
c:\ws\w32-1\llvm-project\premerge-checks\build\bin\clang.exe -cc1
-internal-isystem
c:\ws\w32-1\llvm-project\premerge-checks\build\lib\clang\17\include
-nostdsysteminc -std=c++1z
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\cxx1z-constexpr-if.cpp
-emit-llvm -fblocks -o - |
c:\ws\w32-1\llvm-project\premerge-checks\build\bin\filecheck.exe
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\cxx1z-constexpr-if.cpp
--implicit-check-not=should_not_be_used
--
Exit Code: 1
Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "c:\ws\w32-1\llvm-project\premerge-checks\build\bin\clang.exe" "-cc1"
"-internal-isystem"
"c:\ws\w32-1\llvm-project\premerge-checks\build\lib\clang\17\include"
"-nostdsysteminc" "-std=c++1z"
"C:\ws\w32-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\cxx1z-constexpr-if.cpp"
"-emit-llvm" "-fblocks" "-o" "-"
$ "c:\ws\w32-1\llvm-project\premerge-checks\build\bin\filecheck.exe"
"C:\ws\w32-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\cxx1z-constexpr-if.cpp"
"--implicit-check-not=should_not_be_used"
# command stderr:
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\cxx1z-constexpr-if.cpp:50:17:
error: CHECK-LABEL: expected string not found in input
// CHECK-LABEL: define internal void
@___ZN16BlockThisCapture1S1mILb0EEEvv_block_invoke(
^
:1:1: note: scanning from here
; ModuleID =
'C:\ws\w32-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\cxx1z-constexpr-if.cpp'
^
:53:1: note: possible intended match here
define internal void
@"__??$m@$0A@@S@BlockThisCapture@@QEAAXXZ_block_invoke"(ptr noundef
%.block_descriptor) #3 {
^
Input file:
Check file:
C:\ws\w32-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\cxx1z-constexpr-if.cpp
-dump-input=help explains the following input dump.
Input was:
<<
1: ; ModuleID =
'C:\ws\w32-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\cxx1z-constexpr-if.cpp'
label:50'0
X
error: no match found
2: source_filename =
"C:\\ws\\w32-1\\llvm-project\\premerge-checks\\clang\\test\\CodeGenCXX\\cxx1z-constexpr-if.cpp"
label:50'0
~~
3: target datalayout =
"e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
label:50'0
~
4: target triple = "x86_64-pc-windows-msvc"
label:50'0 ~
5:
label:50'0 ~
6: %"struct.BlockThisCapture::S" = type { i8 }
label:50'0
.
.
.
48: call void %0(ptr noundef @__block_literal_global)
label:50'0 ~~~
49: ret void
label:50'0 ~~
50: }
label:50'0 ~~
51:
label:50'0 ~
52: ; Function Attrs: noinline nounwind optnone
label:50'0
53: define internal void
@"__??$m@$0A@@S@BlockThisCapture@@QEAAXXZ_block_invoke"(ptr noundef
%.block_descriptor) #3 {
label:50'0
~~
label:50'1 ?
possible intended match
54: entry:
label:50'0 ~~~
55: %.block_descriptor.addr = alloca ptr, align 8
label:50'0 ~~~
56: %block.addr = alloca ptr, align 8
label:50'0 ~~~
57: store ptr %.block_descriptor, ptr %.block_descriptor.addr,
align 8
label:50'0
58: store ptr %.block_descriptor, ptr %block.addr, align 8
label:50'0
.
.
.
>>
error: command failed with exit status: 1
It looks like you may need a target triple to ensure the mangled names are
consistent.
Also, can you add a release note for the fix?
Repository:
rG LLVM Github Monorepo
CHANGES
[PATCH] D144016: [Sema] Relax a failing assertion in TransformBlockExpr
ahatanak updated this revision to Diff 497530.
ahatanak added a comment.
Add comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144016/new/
https://reviews.llvm.org/D144016
Files:
clang/lib/Sema/TreeTransform.h
clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
Index: clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s
--implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -fblocks -o - | FileCheck %s
--implicit-check-not=should_not_be_used
void should_be_used_1();
void should_be_used_2();
@@ -32,3 +32,20 @@
// CHECK: should_be_used_1
// CHECK: should_be_used_2
// CHECK: should_be_used_3
+
+namespace BlockThisCapture {
+ void foo();
+ struct S {
+template
+void m() {
+ ^{ if constexpr(b) (void)this; else foo(); }();
+}
+ };
+
+ void test() {
+S().m();
+ }
+}
+
+// CHECK-LABEL: define internal void
@___ZN16BlockThisCapture1S1mILb0EEEvv_block_invoke(
+// CHECK: call void @_ZN16BlockThisCapture3fooEv(
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -14598,7 +14598,12 @@
oldCapture));
assert(blockScope->CaptureMap.count(newCapture));
}
-assert(oldBlock->capturesCXXThis() == blockScope->isCXXThisCaptured());
+
+// The this pointer may not be captured by the instantiated block, even
when
+// it's captured by the original block, if the expression causing the
+// capture is in the discarded branch of a constexpr if statement.
+assert((!blockScope->isCXXThisCaptured() || oldBlock->capturesCXXThis()) &&
+ "this pointer isn't captured in the old block");
}
#endif
Index: clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s --implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -fblocks -o - | FileCheck %s --implicit-check-not=should_not_be_used
void should_be_used_1();
void should_be_used_2();
@@ -32,3 +32,20 @@
// CHECK: should_be_used_1
// CHECK: should_be_used_2
// CHECK: should_be_used_3
+
+namespace BlockThisCapture {
+ void foo();
+ struct S {
+template
+void m() {
+ ^{ if constexpr(b) (void)this; else foo(); }();
+}
+ };
+
+ void test() {
+S().m();
+ }
+}
+
+// CHECK-LABEL: define internal void @___ZN16BlockThisCapture1S1mILb0EEEvv_block_invoke(
+// CHECK: call void @_ZN16BlockThisCapture3fooEv(
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -14598,7 +14598,12 @@
oldCapture));
assert(blockScope->CaptureMap.count(newCapture));
}
-assert(oldBlock->capturesCXXThis() == blockScope->isCXXThisCaptured());
+
+// The this pointer may not be captured by the instantiated block, even when
+// it's captured by the original block, if the expression causing the
+// capture is in the discarded branch of a constexpr if statement.
+assert((!blockScope->isCXXThisCaptured() || oldBlock->capturesCXXThis()) &&
+ "this pointer isn't captured in the old block");
}
#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144016: [Sema] Relax a failing assertion in TransformBlockExpr
ahatanak created this revision.
ahatanak added reviewers: arphaman, fahad, aaron.ballman.
ahatanak added a project: clang.
Herald added a project: All.
ahatanak requested review of this revision.
The assertion fails when the expression causing the this pointer to be captured
is part of a constexpr if statement's branch and the branch gets discarded when
the enclosing method is instantiated.
Note that the test case is added to CodeGen instead of Sema since the
translation unit has to be free of errors in order for the assertion to be
checked.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D144016
Files:
clang/lib/Sema/TreeTransform.h
clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
Index: clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s
--implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -fblocks -o - | FileCheck %s
--implicit-check-not=should_not_be_used
void should_be_used_1();
void should_be_used_2();
@@ -32,3 +32,20 @@
// CHECK: should_be_used_1
// CHECK: should_be_used_2
// CHECK: should_be_used_3
+
+namespace BlockThisCapture {
+ void foo();
+ struct S {
+template
+void m() {
+ ^{ if constexpr(b) (void)this; else foo(); }();
+}
+ };
+
+ void test() {
+S().m();
+ }
+}
+
+// CHECK-LABEL: define internal void
@___ZN16BlockThisCapture1S1mILb0EEEvv_block_invoke(
+// CHECK: call void @_ZN16BlockThisCapture3fooEv(
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -14598,7 +14598,8 @@
oldCapture));
assert(blockScope->CaptureMap.count(newCapture));
}
-assert(oldBlock->capturesCXXThis() == blockScope->isCXXThisCaptured());
+assert((!blockScope->isCXXThisCaptured() || oldBlock->capturesCXXThis()) &&
+ "this pointer isn't captured in the old block");
}
#endif
Index: clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s --implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -fblocks -o - | FileCheck %s --implicit-check-not=should_not_be_used
void should_be_used_1();
void should_be_used_2();
@@ -32,3 +32,20 @@
// CHECK: should_be_used_1
// CHECK: should_be_used_2
// CHECK: should_be_used_3
+
+namespace BlockThisCapture {
+ void foo();
+ struct S {
+template
+void m() {
+ ^{ if constexpr(b) (void)this; else foo(); }();
+}
+ };
+
+ void test() {
+S().m();
+ }
+}
+
+// CHECK-LABEL: define internal void @___ZN16BlockThisCapture1S1mILb0EEEvv_block_invoke(
+// CHECK: call void @_ZN16BlockThisCapture3fooEv(
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -14598,7 +14598,8 @@
oldCapture));
assert(blockScope->CaptureMap.count(newCapture));
}
-assert(oldBlock->capturesCXXThis() == blockScope->isCXXThisCaptured());
+assert((!blockScope->isCXXThisCaptured() || oldBlock->capturesCXXThis()) &&
+ "this pointer isn't captured in the old block");
}
#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
