[PATCH] D140547: Perform access checking to private members in simple requirement.

[Ilya Biryukov via Phabricator via cfe-commits]

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