[libcxx-commits] [clang] [libcxx] [Clang] Implement CWG2369 "Ordering between constraints and substitution" (PR #102857)

[via libcxx-commits]

cor3ntin wrote:

I spent sometimes on this with @zyn0217 

```cpp

template <class, class _Up>
using compare_three_way_result_t = _Up ::type;

struct __sfinae_assign_base {};

template <class _Tp, class _Up>
  requires ([]<class W>(W) { return true; }(_Up())) // #1
compare_three_way_result_t<_Tp, _Up> operator<=>(_Tp, _Up); //#2

struct RuntimeModeArgs {
  auto operator<=>(const RuntimeModeArgs &) const = default;
  __sfinae_assign_base needs_admin;
};
```

In `#1`, the lambda has an `auto` return type. During constraint satisfaction checking (*before* `#2` gets specialized), we need to call `DeduceReturnType` which calls `InstantiateFunctionDefinition`. 

In turn `InstantiateFunctionDefinition` ends up calling `EmitGlobalDecl` through:
https://github.com/llvm/llvm-project/blob/81ae6686ab026c820f2f23d3c18cdd65ecb2ae7c/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L5288

This in turns requires the lambda to have a mangled name, causing its enclosing, dependent function template type to get mangled on the windows abi, which obviously isn't going to work. 

Fortunately, we do not need to mangle constraints, so we just need to find a reasonable way to skip the codegen.
It's weird that we try to emit anything from an unevaluated context to begin with.


This patch does the trick by skipping `EmitGlobalDecl`  from unevaluated and immediate contexts

```diff
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 18fd95f77ec2..0e30f21ab68b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13721,7 +13721,8 @@ public:
                                      FunctionDecl *Function,
                                      bool Recursive = false,
                                      bool DefinitionRequired = false,
-                                     bool AtEndOfTU = false);
+                                     bool AtEndOfTU = false,
+                                     bool DeducingReturnType = false);
   VarTemplateSpecializationDecl *BuildVarTemplateInstantiation(
       VarTemplateDecl *VarTemplate, VarDecl *FromVar,
       const TemplateArgumentList *PartialSpecArgs,
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index acd1151184e4..b9918c556443 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5459,7 +5459,7 @@ bool Sema::DeduceReturnType(FunctionDecl *FD, SourceLocation Loc,
 
   if (FD->getTemplateInstantiationPattern()) {
     runWithSufficientStackSpace(Loc, [&] {
-      InstantiateFunctionDefinition(Loc, FD);
+      InstantiateFunctionDefinition(Loc, FD, false, false, false, /*DeducingReturnType=*/true);
     });
   }
 
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index e058afe81da5..e3d34c305429 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4955,7 +4955,8 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
                                          FunctionDecl *Function,
                                          bool Recursive,
                                          bool DefinitionRequired,
-                                         bool AtEndOfTU) {
+                                         bool AtEndOfTU,
+                                         bool DeducingReturnType) {
   if (Function->isInvalidDecl() || isa<CXXDeductionGuideDecl>(Function))
     return;
 
@@ -5284,8 +5285,11 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
     savedContext.pop();
   }
 
-  DeclGroupRef DG(Function);
-  Consumer.HandleTopLevelDecl(DG);
+  if(!DeducingReturnType || !(parentEvaluationContext().isUnevaluated()
+       || parentEvaluationContext().isImmediateFunctionContext())) {
+     DeclGroupRef DG(Function);
+     Consumer.HandleTopLevelDecl(DG);
+  }
 
   // This class may have local implicit instantiations that need to be
   // instantiation within this scope.
```


The `DeducingReturnType` boolean is only useful not to break 
`CodeGenCXX/template-param-objects-address-space.cpp` so maybe we'd need to investigate that test further
 


@erichkeane @zygoloid @AaronBallman 

 

https://github.com/llvm/llvm-project/pull/102857