[PATCH] D140876: [clang][C++20] Non-dependent access checks in requires expression.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Should access checks should happen in the context where `concept` is written or where it's used? Is there a standard wording around it?
If access-checking should happen where concept is defined, having a hard error seems fine because of the wording you quoted:

> If the substitution of template arguments into a requirement would always result in a substitution failure, the program is ill-formed; no diagnostic required.

The program is ill-formed and we show the diagnostic even though it's not required.

I poked around and found an interesting case where GCC seems to be doing the wrong thing:

  template <class> struct B;
  class A {
     static void f();
     friend struct B<short>;
  };
   
  template <class T> struct B {
      static constexpr int index() requires requires{ A::f(); } {
          return 1;
      }
      static constexpr int index() {
          return 2;
      }
  };
  
  static_assert(B<short>::index() == 1); // GCC picks 1, MSVC picks 1.
  static_assert(B<int>::index() == 2);   // GCC (incorrectly?) picks 1, MSVC picks 2!

Is this related to this change? Could we add a test that validates Clang is doing the right thing?



================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:3510
       Actions, Sema::ExpressionEvaluationContext::Unevaluated);
+  Sema::ContextRAII SaveContext(Actions, Actions.CurContext);
 
----------------
Could you explain how this changes behaviour and how it leads to fixing the issue?

I'm sure there is quite a bit of thought and debugging behind this one-line change, but it's not evident by just looking at it how it solves the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140876