ilya-biryukov added inline comments.

================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1367
+      if (E->getBody()->isDependentContext()) {
+        Sema::SFINAETrap Trap(SemaRef);
+        // We recreate the RequiresExpr body, but not by instantiating it.
----------------
Other uses of `PerformDependentDiagnostics` do not add an explicit `SFINAETrap` 
AFAICS.
Why is `RequiresExpr` special? Because it should "eat" the errors and only 
return a value?


================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1374
+      }
       LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
       return inherited::TransformRequiresExpr(E);
----------------
Other uses of `PerformDependentDiagnostics` happen inside 
`LocalInstantiationScope` and after substitution of inner parts.

I suggest following this pattern and moving the added code after the call to 
`inherited::TransformRequiresExpr`.
I do not have any concrete examples where the current approach fails, but it's 
better to have a single mode of operation across all opeartions.


================
Comment at: 
clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp:157
+static_assert(A<0>::faz());
+}
----------------
Could you add a check that in the following case we mention access check in the 
note to the `no matching function to call` error?

```
template <class T> struct Use;

class X { int a; friend struct Use<short>; };

template <class T> struct Use {
  static void foo() requires (requires (X x) { x.a; }) {
  }
};

void test() {
    Use<int>::foo();
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140547/new/

https://reviews.llvm.org/D140547

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to