usaxena95 updated this revision to Diff 486604.
usaxena95 added a comment.

Use ParsingDeclRAIIObject instead of ContextRAII.

This creates a separate diagnostic pool for diagnositcs associated to the 
RequiresExprBodyDecl.
This is important because dependent diagnostics should not leak to higher 
scopes (Eg. inside a template function or in a trailing requires). These 
dependent diagnstics must be attached to the DeclContext of the parameters of 
RequiresExpr (which is the BodyDecl in this case).
Non dependent diagnostics should not delayed and surfaced as hard errors.

This addresses the previously failing LibCXX failure as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140547

Files:
  clang/include/clang/AST/ExprConcepts.h
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaAccess.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp

Index: clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
===================================================================
--- clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
@@ -104,3 +104,138 @@
 constexpr bool b = requires (X &x) { static_cast<int(*)[(typeid(x), 0)]>(nullptr); };
 // expected-error@-1{{constraint variable 'x' cannot be used in an evaluated context}}
 // expected-note@-2{{'x' declared here}}
+
+namespace access_checks {
+namespace in_requires_expression {
+template<auto>
+struct A {
+    static constexpr bool foo();
+    static constexpr bool bar();
+    static constexpr bool baz();
+    static constexpr bool faz();
+};
+
+class C{};
+
+class B {
+    void p() {}
+    bool data_member = true;
+    static const bool static_member = true;
+    friend struct A<0>;
+};
+
+template<auto x>
+constexpr bool A<x>::foo() {
+    return requires(B b) { b.p(); };
+}
+static_assert(!A<1>::foo());
+static_assert(A<0>::foo());
+
+template<auto x>
+constexpr bool A<x>::bar() {
+    return requires() { B::static_member; };
+}
+static_assert(!A<1>::bar());
+static_assert(A<0>::bar());
+
+template<auto x>
+constexpr bool A<x>::baz() {
+    return requires(B b) { b.data_member; };
+}
+static_assert(!A<1>::baz());
+static_assert(A<0>::baz());
+
+template<auto x>
+constexpr bool A<x>::faz() {
+    return requires(B a, B b) { 
+      a.p();
+      b.data_member;
+      B::static_member;
+    };
+}
+static_assert(!A<1>::faz());
+static_assert(A<0>::faz());
+} // namespace in_requires_expression
+
+namespace in_concepts {
+// Dependent access does not cause hard errors.
+template<int N> class A;
+
+template <> class A<0> {
+  static void f() {}
+};
+template<int N>
+concept C1 = requires() { A<N>::f(); };
+static_assert(!C1<0>);
+
+template <> class A<1> {
+public: 
+  static void f() {}
+};
+static_assert(C1<1>);
+
+// Non-dependent access to private member is a hard error.
+class B{
+   static void f() {} // expected-note 2{{implicitly declared private here}}
+};
+template<class T>
+concept C2 = requires() { B::f(); }; // expected-error {{'f' is a private member}}
+
+constexpr bool non_template_func() {
+  return requires() {
+      B::f(); // expected-error {{'f' is a private member}}
+  };
+}
+template<int x>
+constexpr bool template_func() {
+  return requires() {
+      A<x>::f();
+  };
+}
+static_assert(!template_func<0>());
+static_assert(template_func<1>());
+} // namespace in_concepts
+
+namespace in_trailing_requires {
+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);
+static_assert(B<int>::index() == 2);
+
+namespace missing_member_function {
+template <class T> struct Use;
+class X { 
+  int a;
+  static int B;
+  friend struct Use<short>;
+};
+template <class T> struct Use {
+  constexpr static int foo() requires requires(X x) { x.a; } {
+    return 1;
+  }
+  constexpr static int bar() requires requires { X::B; } {
+    return 1;
+  }
+};
+
+void test() {
+  // TODO: Propagate diagnostic.
+  Use<int>::foo(); //expected-error {{invalid reference to function 'foo': constraints not satisfied}}
+  static_assert(Use<short>::foo() == 1);
+}
+} // namespace missing_member_function
+} // namespace in_trailing_requires
+} // namespace access_check
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ASTMutationListener.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprConcepts.h"
@@ -1363,7 +1364,17 @@
 
     ExprResult TransformRequiresExpr(RequiresExpr *E) {
       LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
-      return inherited::TransformRequiresExpr(E);
+      auto TransReq = inherited::TransformRequiresExpr(E);
+      if (E->getBody()->isDependentContext()) {
+        Sema::SFINAETrap Trap(SemaRef);
+        // We recreate the RequiresExpr body, but not by instantiating it.
+        // Produce pending diagnostics for dependent access check.
+        SemaRef.PerformDependentDiagnostics(E->getBody(), TemplateArgs);
+        // TODO(usx): Store SFINAE diagnostics in RequiresExpr for diagnosis.
+        if (Trap.hasErrorOccurred())
+          TransReq.getAs<RequiresExpr>()->setSatisfied(false);
+      }
+      return TransReq;
     }
 
     bool TransformRequiresExprRequirements(
Index: clang/lib/Sema/SemaConcept.cpp
===================================================================
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -1002,6 +1002,7 @@
     S.DiagnoseUnsatisfiedConstraint(CSE->getSatisfaction());
     return;
   } else if (auto *RE = dyn_cast<RequiresExpr>(SubstExpr)) {
+    // TODO(usx): Store and diagnose dependent diagnositcs here.
     for (concepts::Requirement *Req : RE->getRequirements())
       if (!Req->isDependent() && !Req->isSatisfied()) {
         if (auto *E = dyn_cast<concepts::ExprRequirement>(Req))
Index: clang/lib/Sema/SemaAccess.cpp
===================================================================
--- clang/lib/Sema/SemaAccess.cpp
+++ clang/lib/Sema/SemaAccess.cpp
@@ -1493,6 +1493,8 @@
   } else if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D)) {
     if (isa<DeclContext>(TD->getTemplatedDecl()))
       DC = cast<DeclContext>(TD->getTemplatedDecl());
+  } else if (auto *RD = dyn_cast<RequiresExprBodyDecl>(D)) {
+    DC = RD;
   }
 
   EffectiveContext EC(DC);
Index: clang/lib/Parse/ParseExprCXX.cpp
===================================================================
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3509,6 +3509,7 @@
       Actions, Sema::ExpressionEvaluationContext::Unevaluated);
 
   ParseScope BodyScope(this, Scope::DeclScope);
+  ParsingDeclRAIIObject ParsingBodyDecl(*this, ParsingDeclRAIIObject::NoParent);
   RequiresExprBodyDecl *Body = Actions.ActOnStartRequiresExpr(
       RequiresKWLoc, LocalParameterDecls, getCurScope());
 
@@ -3746,6 +3747,7 @@
   }
   Braces.consumeClose();
   Actions.ActOnFinishRequiresExpr();
+  ParsingBodyDecl.complete(Body);
   return Actions.ActOnRequiresExpr(RequiresKWLoc, Body, LocalParameterDecls,
                                    Requirements, Braces.getCloseLocation());
 }
Index: clang/include/clang/AST/ExprConcepts.h
===================================================================
--- clang/include/clang/AST/ExprConcepts.h
+++ clang/include/clang/AST/ExprConcepts.h
@@ -528,6 +528,12 @@
     return RequiresExprBits.IsSatisfied;
   }
 
+  void setSatisfied(bool IsSatisfied) {
+    assert(!isValueDependent() &&
+           "setSatisfied called on a dependent RequiresExpr");
+    RequiresExprBits.IsSatisfied = IsSatisfied;
+  }
+
   SourceLocation getRequiresKWLoc() const {
     return RequiresExprBits.RequiresKWLoc;
   }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to