[clang] Implement P0692 "Access Checking on Specializations"

[AntonBikineev]

Summary

This is an attempt to implement P0692
Please note a couple of things:

1. given that clang already suppresses access checks on explicit specializations of classes as an extension, I decided to not make the feature C++17-specific and backport it to previous standards. Not sure that this is right though;
2. I'm also not sure that ParseOptionalCXXScopeSpecifer in a declarator is the correct place for suppressing checks.

[rsmith]

How do you avoid suppressing diagnostics in the declaration of a template that is not a partial specialization? For example:

class A { class B {}; };
template<typename T> A::B x; // error, but...
template<typename T> A::B x<T*>; // ok!

I would expect you'd need to delay the access checking diagnostics here until the declaration is complete and you find out whether you have a partial specialization or not.

[AntonBikineev]

@rsmith Thanks for pointing out this example. Now I see that I misunderstood the wording.
Another question is that in the provided example you say that the following line

template<typename T> A::B x<T*>; // ok!

should suppress the error of accessing private A::B. But in the wording it's said that

The usual access checking rules do not apply to non-dependent names used to specify template arguments of the simple-template-id of the partial specialization.

So it gave me the impression that only names used as template arguments should be exempted from checks. But here A::B is a non-dependent name that's part of the declaration-specifier. So is my impression wrong and names in declaration-specifiers/declarators of partial-specialiations of variable-templates should be exempted from checks as well?

Thanks.

[rsmith]

In D43153#1004464, @AntonBikineev wrote:

So is my impression wrong and names in declaration-specifiers/declarators of partial-specialiations of variable-templates should be exempted from checks as well?

No, sorry, that's my error not yours.

[lebedev.ri]

This review seems to be stuck/dead, consider abandoning if no longer relevant.

include/clang/Parse/RAIIObjectsForParser.h

namespace clang {
 
   /// \brief A RAII object used to temporarily suppress access-like
   /// checking.  Access-like checks are those associated with
   /// controlling the use of a declaration, like C++ access control
   /// errors and deprecation warnings.  They are contextually
   /// dependent, in that they can only be resolved with full
   /// information about what's being declared.  They are also
   /// suppressed in certain contexts, like the template arguments of
-  /// an explicit instantiation.  However, those suppression contexts
+  /// an explicit instantiation and an explicit/partial
+  /// specialization. However, those suppression contexts
   /// cannot necessarily be fully determined in advance;  for
   /// example, something starting like this:
   ///   template <> class std::vector<A::PrivateType>
   /// might be the entirety of an explicit instantiation:
   ///   template <> class std::vector<A::PrivateType>;
   /// or just an elaborated type specifier:
   ///   template <> class std::vector<A::PrivateType> make_vector<>();
   /// Therefore this class collects all the diagnostics and permits

include/clang/Sema/DeclSpec.h

private:
   unsigned ObjCIvar : 1;
     
   /// Indicates whether this is an Objective-C 'weak' property.
   unsigned ObjCWeakProperty : 1;
 
   /// Indicates whether the InlineParams / InlineBindings storage has been used.
   unsigned InlineStorageUsed : 1;
 
+  /// Indicates whether the declarator is part of a
+  /// function/variable template-declaration or an explicit-specialization
+  unsigned TemplateDeclOrSpec : 1;
+
   /// Attrs - Attributes.
   ParsedAttributes Attrs;
 
   /// \brief The asm label, if specified.
   Expr *AsmLabel;
 
 #ifndef _MSC_VER
   union {

 
 public:
   Declarator(const DeclSpec &ds, DeclaratorContext C)
       : DS(ds), Range(ds.getSourceRange()), Context(C),
         InvalidType(DS.getTypeSpecType() == DeclSpec::TST_error),
         GroupingParens(false), FunctionDefinition(FDK_Declaration),
         Redeclaration(false), Extension(false), ObjCIvar(false),
         ObjCWeakProperty(false), InlineStorageUsed(false),
-        Attrs(ds.getAttributePool().getFactory()), AsmLabel(nullptr) {}
+        TemplateDeclOrSpec(false), Attrs(ds.getAttributePool().getFactory()),
+        AsmLabel(nullptr) {}
 
   ~Declarator() {
     clear();
   }
   /// getDeclSpec - Return the declaration-specifier that this declarator was
   /// declared with.
   const DeclSpec &getDeclSpec() const { return DS; }
 
void clear() {
     BindingGroup.clear();
 
     for (unsigned i = 0, e = DeclTypeInfo.size(); i != e; ++i)
       DeclTypeInfo[i].destroy();
     DeclTypeInfo.clear();
     Attrs.clear();
     AsmLabel = nullptr;
     InlineStorageUsed = false;
+    TemplateDeclOrSpec = false;
     ObjCIvar = false;
     ObjCWeakProperty = false;
     CommaLoc = SourceLocation();
     EllipsisLoc = SourceLocation();
   }
 
   /// mayOmitIdentifier - Return true if the identifier is either optional or
   /// not allowed.  This is true for typenames, prototypes, and template
public:
   bool getExtension() const { return Extension; }
 
   void setObjCIvar(bool Val = true) { ObjCIvar = Val; }
   bool isObjCIvar() const { return ObjCIvar; }
     
   void setObjCWeakProperty(bool Val = true) { ObjCWeakProperty = Val; }
   bool isObjCWeakProperty() const { return ObjCWeakProperty; }
 
+  void setTemplateDeclOrSpec(bool Val = true) { TemplateDeclOrSpec = Val; }
+  bool isTemplateDeclOrSpec() const { return TemplateDeclOrSpec; }
+
   void setInvalidType(bool Val = true) { InvalidType = Val; }
   bool isInvalidType() const {
     return InvalidType || DS.getTypeSpecType() == DeclSpec::TST_error;
   }
 
   void setGroupingParens(bool flag) { GroupingParens = flag; }
   bool hasGroupingParens() const { return GroupingParens; }
 

lib/Parse/ParseDecl.cpp

if (Tok.isOneOf(tok::kw_class, tok::kw_struct)) {
     MaybeParseGNUAttributes(attrs);
     MaybeParseCXX11Attributes(attrs);
     MaybeParseMicrosoftDeclSpecs(attrs);
   }
 
   // C++11 [temp.explicit]p12:
   //   The usual access controls do not apply to names used to specify
   //   explicit instantiations.
-  // We extend this to also cover explicit specializations.  Note that
-  // we don't suppress if this turns out to be an elaborated type
-  // specifier.
+  //
+  // C++2a [temp.spec] 17.8/6:
+  //   The usual access checking rules do not apply to names in a declaration
+  //   of an explicit instantiation or explicit specialization, with the
+  //   exception of names appearing in a function body, default argument,
+  //   base-clause, member-specification, enumerator-list,
+  //   or static data member or variable template initializer
+  //
+  // C++2a [temp.class.spec] 17.6.5/10:
+  //   The usual access checking rules do not apply to non-dependent names
+  //   used to specify template arguments of the simple-template-id
+  //   of the partial specialization
+  //
+  // Note that we don't suppress if this turns out to be an elaborated
+  // type specifier.
   bool shouldDelayDiagsInTag =
-    (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation ||
-     TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
+    TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate;
   SuppressAccessChecks diagsFromTag(*this, shouldDelayDiagsInTag);
 
   // Enum definitions should not be parsed in a trailing-return-type.
   bool AllowDeclaration = DSC != DeclSpecContext::DSC_trailing;
 
   bool AllowFixedUnderlyingType = AllowDeclaration &&
     (getLangOpts().CPlusPlus11 || getLangOpts().MicrosoftExt ||
      getLangOpts().ObjC2);
void Parser::ParseDeclaratorInternal(Declarator &D,
   // C++ member pointers start with a '::' or a nested-name.
   // Member pointers get special handling, since there's no place for the
   // scope spec in the generic path below.
   if (getLangOpts().CPlusPlus &&
       (Tok.is(tok::coloncolon) || Tok.is(tok::kw_decltype) ||
        (Tok.is(tok::identifier) &&
         (NextToken().is(tok::coloncolon) || NextToken().is(tok::less))) ||
        Tok.is(tok::annot_cxxscope))) {
+    SuppressAccessChecks diagsFromDeclr(*this, D.isTemplateDeclOrSpec());
+
     bool EnteringContext =
         D.getContext() == DeclaratorContext::FileContext ||
         D.getContext() == DeclaratorContext::MemberContext;
     CXXScopeSpec SS;
     ParseOptionalCXXScopeSpecifier(SS, nullptr, EnteringContext);
 
+    if (D.isTemplateDeclOrSpec())
+      diagsFromDeclr.done();
+
     if (SS.isNotEmpty()) {
       if (Tok.isNot(tok::star)) {
         // The scope spec really belongs to the direct-declarator.
         if (D.mayHaveIdentifier())
           D.getCXXScopeSpec() = SS;
         else
           AnnotateScopeToken(SS, true);
 

lib/Parse/ParseDeclCXX.cpp

if (Tok.is(tok::code_completion)) {
     Actions.CodeCompleteTag(getCurScope(), TagType);
     return cutOffParsing();
   }
 
   // C++03 [temp.explicit] 14.7.2/8:
   //   The usual access checking rules do not apply to names used to specify
   //   explicit instantiations.
   //
-  // As an extension we do not perform access checking on the names used to
-  // specify explicit specializations either. This is important to allow
-  // specializing traits classes for private types.
+  // C++2a [temp.spec] 17.8/6:
+  //   The usual access checking rules do not apply to names in a declaration
+  //   of an explicit instantiation or explicit specialization, with the
+  //   exception of names appearing in a function body, default argument,
+  //   base-clause, member-specification, enumerator-list,
+  //   or static data member or variable template initializer
+  //
+  // C++2a [temp.class.spec] 17.6.5/10:
+  //   The usual access checking rules do not apply to non-dependent names
+  //   used to specify template arguments of the simple-template-id
+  //   of the partial specialization
   //
   // Note that we don't suppress if this turns out to be an elaborated
   // type specifier.
   bool shouldDelayDiagsInTag =
-    (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation ||
-     TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
+    TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate;
   SuppressAccessChecks diagsFromTag(*this, shouldDelayDiagsInTag);
 
   ParsedAttributesWithRange attrs(AttrFactory);
   // If attributes exist after tag, parse them.
   MaybeParseGNUAttributes(attrs);
   MaybeParseMicrosoftDeclSpecs(attrs);
 
   // Parse inheritance specifiers.

lib/Parse/ParseTemplate.cpp

Parser::ParseSingleDeclarationAfterTemplate(
   // Move the attributes from the prefix into the DS.
   if (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation)
     ProhibitAttributes(prefixAttrs);
   else
     DS.takeAttributesFrom(prefixAttrs);
 
   // Parse the declarator.
   ParsingDeclarator DeclaratorInfo(*this, DS, (DeclaratorContext)Context);
+  DeclaratorInfo.setTemplateDeclOrSpec(TemplateInfo.Kind !=
+                                       ParsedTemplateInfo::NonTemplate);
   ParseDeclarator(DeclaratorInfo);
+
   // Error parsing the declarator?
   if (!DeclaratorInfo.hasName()) {
     // If so, skip until the semi-colon or a }.
     SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
     if (Tok.is(tok::semi))
       ConsumeToken();
     return nullptr;
   }

test/CXX/drs/dr1xx.cpp

template <class T> struct C {
     void f();
     void g();
   };
 
   template <class T> void C<T>::f() {}
   template <class T> void C<T>::g() {}
 
   class A {
-    class B {}; // expected-note {{here}}
+    class B {};
     void f();
   };
 
   template void C<A::B>::f();
-  template <> void C<A::B>::g(); // expected-error {{private}}
+  template <> void C<A::B>::g();
 
   void A::f() {
     C<B> cb;
     cb.f();
   }
 }
 
 namespace dr183 { // dr183: sup 382

test/SemaCXX/access.cpp

-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
 
 class C {
     struct S; // expected-note {{previously declared 'private' here}}
 public:
     
     struct S {}; // expected-error {{'S' redeclared with 'public' access}}
 };
 
class A {
     friend class D;
     static void foo(); // expected-note {{here}}
   };
   template <class T> void foo() {
     []() { A::foo(); }(); // expected-error {{private}}
   }
   void bar() { foo<void>(); }
 }
+
+namespace P0692 {
+  namespace class_ {
+    template <class>
+    class trait_type {};
+   
+    template <int>
+    class trait_nontype;
+
+    template <template <class> class>
+    class trait_template;
+
+    class A {
+      class B1 {}; // expected-note 2{{here}}
+
+      template <class>
+      class B2 {}; // expected-note 2{{here}}
+
+      class B3 {};
+   
+      static const int I = 0; // expected-note 4{{here}}
+    };
+   
+    template <class T>
+    class trait_type<A::B2<T>> {
+      void use_private(int = A::I) { // expected-error {{private}}
+        A::B2<int> error; // expected-error {{private}}
+      }
+    };
+
+    template <template <int> class T>
+    class trait_type<T<A::I>> {
+      static const int I = A::I; // expected-error {{private}}
+    };
+   
+    template <>
+    class trait_type<A::B1>
+      : A::B1 { // expected-error {{private}}
+      void use_private(int = A::I) { // expected-error {{private}}
+        A::B1 error; // expected-error {{private}}
+      }
+    };
+
+    template class trait_type<A::B3>;
+
+    template <>
+    class trait_nontype<A::I> {
+      static const int I = A::I; // expected-error {{private}}
+    };
+
+    template <>
+    class trait_template<A::B2> {
+      A::B2<int> error; // expected-error {{private}}
+    };
+  }
+
+  namespace function {
+    class A {
+      class B1 {}; // expected-note 1{{here}}
+
+      class B2 {};
+   
+      using Int = int; // expected-note 1{{here}}
+    };
+   
+    template <class T>
+    void foo1() {}
+
+    template <> void foo1<A::B1>() {
+        A::B1 error; // expected-error {{private}}
+    }
+
+    template void foo1<A::B2>();
+
+    template <class T>
+    void foo2(A::Int) {} // expected-error {{private}}
+  }
+
+  namespace variable {
+    class A {
+      class B1 {};
+
+      template <class>
+      class B2 {};
+   
+      static const int I = 0; // expected-note 2{{here}}
+    };
+
+    template <class T>
+    int var = 0;
+
+    template <>
+    int var<A::B1> =
+        A::I; // expected-error {{private}}
+
+    template <class T>
+    int var<A::B2<T>> =
+        A::I; // expected-error {{private}}
+  }
+}