[Sema] Relax a failing assertion in TransformBlockExpr

[ahatanak]

Summary

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.

[ahatanak]

Add comment.

[aaron.ballman]

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(
                ^
<stdin>:1:1: note: scanning from here
; ModuleID = 'C:\ws\w32-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\cxx1z-constexpr-if.cpp'
^
<stdin>:53:1: note: possible intended match here
define internal void @"__??$m@$0A@@S@BlockThisCapture@@QEAAXXZ_block_invoke"(ptr noundef %.block_descriptor) #3 {
^

Input file: <stdin>
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?

[ahatanak]

Add triple to the test and add release note for the fix.

clang/docs/ReleaseNotes.rst

 
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates.
   (`#60344 <https://github.com/llvm/llvm-project/issues/60344>`_)
 - Fix confusing warning message when ``/clang:-x`` is passed in ``clang-cl``
   driver mode and emit an error which suggests using ``/TC`` or ``/TP``
   ``clang-cl`` options instead.
   (`#59307 <https://github.com/llvm/llvm-project/issues/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
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to C++ Support

clang/lib/Sema/TreeTransform.h

for (const auto &I : oldBlock->captures()) {
       if (oldCapture->isParameterPack())
         continue;
 
       VarDecl *newCapture =
         cast<VarDecl>(getDerived().TransformDecl(E->getCaretLocation(),
                                                  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
 
   return SemaRef.ActOnBlockStmtExpr(E->getCaretLocation(), body.get(),
                                     /*Scope=*/nullptr);
 }
 
 template<typename Derived>

clang/test/CodeGenCXX/cxx1z-constexpr-if.cpp

-// 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();
 void should_be_used_3();
 void should_not_be_used();
 
 struct A {
   constexpr explicit operator bool() const {
if constexpr (A())
     should_be_used_3();
   else
     should_not_be_used();
 }
 
 // CHECK: should_be_used_1
 // CHECK: should_be_used_2
 // CHECK: should_be_used_3
+
+namespace BlockThisCapture {
+  void foo();
+  struct S {
+    template <bool b>
+    void m() {
+      ^{ if constexpr(b) (void)this; else foo();  }();
+    }
+  };
+
+  void test() {
+    S().m<false>();
+  }
+}
+
+// CHECK-LABEL: define internal void @___ZN16BlockThisCapture1S1mILb0EEEvv_block_invoke(
+// CHECK: call void @_ZN16BlockThisCapture3fooEv(