Author: Utkarsh Saxena Date: 2023-01-11T12:13:16+01:00 New Revision: 9e0474fbb9c56725a1dfd1658837f07db73f4d8d
URL: https://github.com/llvm/llvm-project/commit/9e0474fbb9c56725a1dfd1658837f07db73f4d8d DIFF: https://github.com/llvm/llvm-project/commit/9e0474fbb9c56725a1dfd1658837f07db73f4d8d.diff LOG: Perform access checking to private members in simple requirement. > Dependent access checks. Fixes: https://github.com/llvm/llvm-project/issues/53364 We previously ignored dependent access checks to private members. These are visible only to the `RequiresExprBodyExpr` (through `PerformDependentDiagnositcs`) and not to the individual requirements. --- > Non-dependent access checks. Fixes: https://github.com/llvm/llvm-project/issues/53334 Access to members in a non-dependent context would always yield an invalid expression. When it appears in a requires-expression, then this is a hard error as this would always result in a substitution failure. https://eel.is/c++draft/expr.prim.req#general-note-1 > Note 1: If a requires-expression contains invalid types or expressions in its > requirements, and it does not appear within the declaration of a templated > entity, then the program is ill-formed. — end note] > 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 main issue here is the delaying of the diagnostics. Use a `ParsingDeclRAIIObject` creates a separate diagnostic pool for diagnositcs associated to the `RequiresExprBodyDecl`. This is important because dependent diagnostics should not be leaked/delayed to higher scopes (Eg. inside a template function or in a trailing requires). These dependent diagnostics must be attached to the `DeclContext` of the parameters of `RequiresExpr` (which is the `RequiresExprBodyDecl` in this case). Non dependent diagnostics, on the other hand, should not delayed and surfaced as hard errors. Differential Revision: https://reviews.llvm.org/D140547 Added: clang/test/SemaCXX/invalid-requirement-requires-expr.cpp Modified: clang/docs/ReleaseNotes.rst 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 Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 927c85249b729..b55b2d6752972 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -735,7 +735,8 @@ C++20 Feature Support - Do not hide templated base members introduced via using-decl in derived class (useful specially for constrained members). Fixes `GH50886 <https://github.com/llvm/llvm-project/issues/50886>`_. - Implemented CWG2635 as a Defect Report, which prohibits structured bindings from being constrained. - +- Correctly handle access-checks in requires expression. Fixes `GH53364 <https://github.com/llvm/llvm-project/issues/53364>`_, + `GH53334 <https://github.com/llvm/llvm-project/issues/53334>`_. C++2b Feature Support ^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/ExprConcepts.h b/clang/include/clang/AST/ExprConcepts.h index a358a2a41619e..f02c140c14c19 100644 --- a/clang/include/clang/AST/ExprConcepts.h +++ b/clang/include/clang/AST/ExprConcepts.h @@ -528,6 +528,12 @@ class RequiresExpr final : public Expr, return RequiresExprBits.IsSatisfied; } + void setSatisfied(bool IsSatisfied) { + assert(!isValueDependent() && + "setSatisfied called on a dependent RequiresExpr"); + RequiresExprBits.IsSatisfied = IsSatisfied; + } + SourceLocation getRequiresKWLoc() const { return RequiresExprBits.RequiresKWLoc; } diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp index 5017f31a1ecb2..7f09120574a7a 100644 --- a/clang/lib/Parse/ParseExprCXX.cpp +++ b/clang/lib/Parse/ParseExprCXX.cpp @@ -3509,6 +3509,10 @@ ExprResult Parser::ParseRequiresExpression() { Actions, Sema::ExpressionEvaluationContext::Unevaluated); ParseScope BodyScope(this, Scope::DeclScope); + // Create a separate diagnostic pool for RequiresExprBodyDecl. + // Dependent diagnostics are attached to this Decl and non-depenedent + // diagnostics are surfaced after this parse. + ParsingDeclRAIIObject ParsingBodyDecl(*this, ParsingDeclRAIIObject::NoParent); RequiresExprBodyDecl *Body = Actions.ActOnStartRequiresExpr( RequiresKWLoc, LocalParameterDecls, getCurScope()); @@ -3746,6 +3750,7 @@ ExprResult Parser::ParseRequiresExpression() { } Braces.consumeClose(); Actions.ActOnFinishRequiresExpr(); + ParsingBodyDecl.complete(Body); return Actions.ActOnRequiresExpr(RequiresKWLoc, Body, LocalParameterDecls, Requirements, Braces.getCloseLocation()); } diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp index 00d3efd19d7a2..cbda62497e6a6 100644 --- a/clang/lib/Sema/SemaAccess.cpp +++ b/clang/lib/Sema/SemaAccess.cpp @@ -1493,6 +1493,8 @@ void Sema::HandleDelayedAccessCheck(DelayedDiagnostic &DD, Decl *D) { } 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); diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index cab013de95c12..af920693859e4 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -1002,6 +1002,7 @@ static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S, S.DiagnoseUnsatisfiedConstraint(CSE->getSatisfaction()); return; } else if (auto *RE = dyn_cast<RequiresExpr>(SubstExpr)) { + // FIXME: RequiresExpr should store dependent diagnostics. for (concepts::Requirement *Req : RE->getRequirements()) if (!Req->isDependent() && !Req->isSatisfied()) { if (auto *E = dyn_cast<concepts::ExprRequirement>(Req)) diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 97571837e6148..b43a754a58ce7 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/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" @@ -1362,7 +1363,22 @@ namespace { ExprResult TransformRequiresExpr(RequiresExpr *E) { LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true); - return inherited::TransformRequiresExpr(E); + auto TransReq = inherited::TransformRequiresExpr(E); + if (TransReq.isInvalid()) + return TransReq; + assert(TransReq.get() != E && + "Do not change value of isSatisfied for the existing expression. " + "Create a new expression instead."); + 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); + // FIXME: Store SFINAE diagnostics in RequiresExpr for diagnosis. + if (Trap.hasErrorOccurred()) + TransReq.getAs<RequiresExpr>()->setSatisfied(false); + } + return TransReq; } bool TransformRequiresExprRequirements( diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp b/clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp index e491e039026b8..abfadfa348841 100644 --- a/clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp +++ b/clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp @@ -104,3 +104,138 @@ class X { virtual ~X(); }; 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() { + // FIXME: 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 diff --git a/clang/test/SemaCXX/invalid-requirement-requires-expr.cpp b/clang/test/SemaCXX/invalid-requirement-requires-expr.cpp new file mode 100644 index 0000000000000..c23850219c8a8 --- /dev/null +++ b/clang/test/SemaCXX/invalid-requirement-requires-expr.cpp @@ -0,0 +1,23 @@ +// RUN: %clang_cc1 %s -I%S -std=c++2a -verify + +// RequiresExpr contains invalid requirement. (Eg. Highly recurisive template). +template<int x> +struct A { static constexpr bool far(); }; +class B { + bool data_member; + friend struct A<1>; +}; + +template<> +constexpr bool A<0>::far() { return true; } + +template<int x> +constexpr bool A<x>::far() { + return requires(B b) { + b.data_member; + requires A<x-1>::far(); //expected-note 3{{in instantiation}} // expected-note 6{{while}} expected-note {{contexts in backtrace; use -ftemplate-backtrace-limit=0 to see all}} + // expected-error@-1{{recursive template instantiation exceeded maximum depth}} + }; +} +static_assert(A<1>::far()); +static_assert(!A<10001>::far()); // expected-note {{in instantiation of member function}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
