mboehme created this revision.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

I noticed that a lot of functions in the parser take an `AccessAttrs` parameter,
even though, as far as I can tell, it's sufficient to handle access specifier
attributes locally where the access specifier is parsed.

I have removed all of this plumbing, and tests still pass. Much of the code
involved is pretty old, so I suspect this plumbing was required at some point
but that the underlying reason has now been eliminated. It may also be that at
some point people started simply following the pattern of "pass `AccessAttrs`
wherever an `AccessSpecifier` is passed".

However, I would appreciate a close look from someone who's more familiar with
the code than I am.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126066

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp

Index: clang/lib/Parse/Parser.cpp
===================================================================
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -819,7 +819,7 @@
   case tok::annot_attr_openmp:
   case tok::annot_pragma_openmp: {
     AccessSpecifier AS = AS_none;
-    return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, Attrs);
+    return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS);
   }
   case tok::annot_pragma_ms_pointers_to_members:
     HandlePragmaMSPointersToMembers();
@@ -974,7 +974,7 @@
              diag::ext_extern_template) << SourceRange(ExternLoc, TemplateLoc);
       SourceLocation DeclEnd;
       return Actions.ConvertDeclToDeclGroup(ParseExplicitInstantiation(
-          DeclaratorContext::File, ExternLoc, TemplateLoc, DeclEnd, Attrs));
+          DeclaratorContext::File, ExternLoc, TemplateLoc, DeclEnd));
     }
     goto dont_know;
 
Index: clang/lib/Parse/ParseTemplate.cpp
===================================================================
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -35,17 +35,16 @@
 
 /// Parse a template declaration, explicit instantiation, or
 /// explicit specialization.
-Decl *Parser::ParseDeclarationStartingWithTemplate(
-    DeclaratorContext Context, SourceLocation &DeclEnd,
-    ParsedAttributes &AccessAttrs, AccessSpecifier AS) {
+Decl *Parser::ParseDeclarationStartingWithTemplate(DeclaratorContext Context,
+                                                   SourceLocation &DeclEnd,
+                                                   AccessSpecifier AS) {
   ObjCDeclContextSwitch ObjCDC(*this);
 
   if (Tok.is(tok::kw_template) && NextToken().isNot(tok::less)) {
     return ParseExplicitInstantiation(Context, SourceLocation(), ConsumeToken(),
-                                      DeclEnd, AccessAttrs, AS);
+                                      DeclEnd, AS);
   }
-  return ParseTemplateDeclarationOrSpecialization(Context, DeclEnd, AccessAttrs,
-                                                  AS);
+  return ParseTemplateDeclarationOrSpecialization(Context, DeclEnd, AS);
 }
 
 /// Parse a template declaration or an explicit specialization.
@@ -73,8 +72,7 @@
 ///       explicit-specialization: [ C++ temp.expl.spec]
 ///         'template' '<' '>' declaration
 Decl *Parser::ParseTemplateDeclarationOrSpecialization(
-    DeclaratorContext Context, SourceLocation &DeclEnd,
-    ParsedAttributes &AccessAttrs, AccessSpecifier AS) {
+    DeclaratorContext Context, SourceLocation &DeclEnd, AccessSpecifier AS) {
   assert(Tok.isOneOf(tok::kw_export, tok::kw_template) &&
          "Token does not start a template declaration.");
 
@@ -170,7 +168,7 @@
   return ParseSingleDeclarationAfterTemplate(
       Context,
       ParsedTemplateInfo(&ParamLists, isSpecialization, LastParamListWasEmpty),
-      ParsingTemplateParams, DeclEnd, AccessAttrs, AS);
+      ParsingTemplateParams, DeclEnd, AS);
 }
 
 /// Parse a single declaration that declares a template,
@@ -186,7 +184,7 @@
 Decl *Parser::ParseSingleDeclarationAfterTemplate(
     DeclaratorContext Context, const ParsedTemplateInfo &TemplateInfo,
     ParsingDeclRAIIObject &DiagsFromTParams, SourceLocation &DeclEnd,
-    ParsedAttributes &AccessAttrs, AccessSpecifier AS) {
+    AccessSpecifier AS) {
   assert(TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate &&
          "Template information required");
 
@@ -200,8 +198,7 @@
 
   if (Context == DeclaratorContext::Member) {
     // We are parsing a member template.
-    ParseCXXClassMemberDeclaration(AS, AccessAttrs, TemplateInfo,
-                                   &DiagsFromTParams);
+    ParseCXXClassMemberDeclaration(AS, TemplateInfo, &DiagsFromTParams);
     return nullptr;
   }
 
@@ -1640,7 +1637,6 @@
                                          SourceLocation ExternLoc,
                                          SourceLocation TemplateLoc,
                                          SourceLocation &DeclEnd,
-                                         ParsedAttributes &AccessAttrs,
                                          AccessSpecifier AS) {
   // This isn't really required here.
   ParsingDeclRAIIObject
@@ -1648,7 +1644,7 @@
 
   return ParseSingleDeclarationAfterTemplate(
       Context, ParsedTemplateInfo(ExternLoc, TemplateLoc),
-      ParsingTemplateParams, DeclEnd, AccessAttrs, AS);
+      ParsingTemplateParams, DeclEnd, AS);
 }
 
 SourceRange Parser::ParsedTemplateInfo::getSourceRange() const {
Index: clang/lib/Parse/ParseOpenMP.cpp
===================================================================
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2026,8 +2026,7 @@
 ///         annot_pragma_openmp_end
 ///
 Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
-    AccessSpecifier &AS, ParsedAttributes &Attrs, bool Delayed,
-    DeclSpec::TST TagType, Decl *Tag) {
+    AccessSpecifier &AS, bool Delayed, DeclSpec::TST TagType, Decl *Tag) {
   assert(Tok.isOneOf(tok::annot_pragma_openmp, tok::annot_attr_openmp) &&
          "Not an OpenMP directive!");
   ParsingOpenMPDirectiveRAII DirScope(*this);
@@ -2278,18 +2277,18 @@
 
     DeclGroupPtrTy Ptr;
     if (Tok.isOneOf(tok::annot_pragma_openmp, tok::annot_attr_openmp)) {
-      Ptr = ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, Attrs, Delayed,
-                                                       TagType, Tag);
+      Ptr =
+          ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, Delayed, TagType, Tag);
     } else if (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
       // Here we expect to see some function declaration.
       if (AS == AS_none) {
         assert(TagType == DeclSpec::TST_unspecified);
+        ParsedAttributes Attrs(AttrFactory);
         MaybeParseCXX11Attributes(Attrs);
         ParsingDeclSpec PDS(*this);
         Ptr = ParseExternalDeclaration(Attrs, &PDS);
       } else {
-        Ptr =
-            ParseCXXClassMemberDeclarationWithPragmas(AS, Attrs, TagType, Tag);
+        Ptr = ParseCXXClassMemberDeclarationWithPragmas(AS, TagType, Tag);
       }
     }
     if (!Ptr) {
Index: clang/lib/Parse/ParseDeclCXX.cpp
===================================================================
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2573,7 +2573,6 @@
 ///
 Parser::DeclGroupPtrTy
 Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
-                                       ParsedAttributes &AccessAttrs,
                                        const ParsedTemplateInfo &TemplateInfo,
                                        ParsingDeclRAIIObject *TemplateDiags) {
   if (Tok.is(tok::at)) {
@@ -2664,7 +2663,7 @@
     SourceLocation DeclEnd;
     return DeclGroupPtrTy::make(
         DeclGroupRef(ParseTemplateDeclarationOrSpecialization(
-            DeclaratorContext::Member, DeclEnd, AccessAttrs, AS)));
+            DeclaratorContext::Member, DeclEnd, AS)));
   }
 
   // Handle:  member-declaration ::= '__extension__' member-declaration
@@ -2672,8 +2671,7 @@
     // __extension__ silences extension warnings in the subexpression.
     ExtensionRAIIObject O(Diags);  // Use RAII to do this.
     ConsumeToken();
-    return ParseCXXClassMemberDeclaration(AS, AccessAttrs,
-                                          TemplateInfo, TemplateDiags);
+    return ParseCXXClassMemberDeclaration(AS, TemplateInfo, TemplateDiags);
   }
 
   ParsedAttributes attrs(AttrFactory);
@@ -2685,7 +2683,7 @@
   // normally be handled from ParseCXXClassMemberDeclarationWithPragmas, but in
   // this case, it came from an *attribute* rather than a pragma. Handle it now.
   if (Tok.is(tok::annot_attr_openmp))
-    return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, attrs);
+    return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS);
 
   // We need to keep these attributes for future diagnostic
   // before they are taken over by declaration specifier.
@@ -2888,9 +2886,8 @@
         DS.ClearStorageClassSpecs();
       }
 
-      Decl *FunDecl =
-        ParseCXXInlineMethodDef(AS, AccessAttrs, DeclaratorInfo, TemplateInfo,
-                                VS, PureSpecLoc);
+      Decl *FunDecl = ParseCXXInlineMethodDef(AS, DeclaratorInfo, TemplateInfo,
+                                              VS, PureSpecLoc);
 
       if (FunDecl) {
         for (unsigned i = 0, ni = CommonLateParsedAttrs.size(); i < ni; ++i) {
@@ -2975,9 +2972,6 @@
         // Re-direct this decl to refer to the templated decl so that we can
         // initialize it.
         ThisDecl = VT->getTemplatedDecl();
-
-      if (ThisDecl)
-        Actions.ProcessDeclAttributeList(getCurScope(), ThisDecl, AccessAttrs);
     }
 
     // Error recovery might have converted a non-static member into a static
@@ -3219,14 +3213,13 @@
 }
 
 Parser::DeclGroupPtrTy Parser::ParseCXXClassMemberDeclarationWithPragmas(
-    AccessSpecifier &AS, ParsedAttributes &AccessAttrs, DeclSpec::TST TagType,
-    Decl *TagDecl) {
+    AccessSpecifier &AS, DeclSpec::TST TagType, Decl *TagDecl) {
   ParenBraceBracketBalancer BalancerRAIIObj(*this);
 
   switch (Tok.getKind()) {
   case tok::kw___if_exists:
   case tok::kw___if_not_exists:
-    ParseMicrosoftIfExistsClassDeclaration(TagType, AccessAttrs, AS);
+    ParseMicrosoftIfExistsClassDeclaration(TagType, AS);
     return nullptr;
 
   case tok::semi:
@@ -3266,7 +3259,7 @@
     // FIXME: We don't accept GNU attributes on access specifiers in OpenCL mode
     // yet.
     if (getLangOpts().OpenCL && !NextToken().is(tok::colon))
-      return ParseCXXClassMemberDeclaration(AS, AccessAttrs);
+      return ParseCXXClassMemberDeclaration(AS);
     LLVM_FALLTHROUGH;
   case tok::kw_public:
   case tok::kw_protected: {
@@ -3277,7 +3270,7 @@
     SourceLocation ASLoc = Tok.getLocation();
     unsigned TokLength = Tok.getLength();
     ConsumeToken();
-    AccessAttrs.clear();
+    ParsedAttributes AccessAttrs(AttrFactory);
     MaybeParseGNUAttributes(AccessAttrs);
 
     SourceLocation EndLoc;
@@ -3297,18 +3290,15 @@
       Diag(ASLoc, diag::err_access_specifier_interface) << (AS == AS_protected);
     }
 
-    if (Actions.ActOnAccessSpecifier(NewAS, ASLoc, EndLoc, AccessAttrs)) {
-      // found another attribute than only annotations
-      AccessAttrs.clear();
-    }
+    Actions.ActOnAccessSpecifier(NewAS, ASLoc, EndLoc, AccessAttrs);
 
     return nullptr;
   }
 
   case tok::annot_attr_openmp:
   case tok::annot_pragma_openmp:
-    return ParseOpenMPDeclarativeDirectiveWithExtDecl(
-        AS, AccessAttrs, /*Delayed=*/true, TagType, TagDecl);
+    return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, /*Delayed=*/true,
+                                                      TagType, TagDecl);
 
   default:
     if (tok::isPragmaAnnotation(Tok.getKind())) {
@@ -3318,7 +3308,7 @@
       ConsumeAnnotationToken();
       return nullptr;
     }
-    return ParseCXXClassMemberDeclaration(AS, AccessAttrs);
+    return ParseCXXClassMemberDeclaration(AS);
   }
 }
 
@@ -3514,7 +3504,6 @@
     CurAS = AS_private;
   else
     CurAS = AS_public;
-  ParsedAttributes AccessAttrs(AttrFactory);
 
   if (TagDecl) {
     // While we still have something to read, read the member-declarations.
@@ -3522,7 +3511,7 @@
            Tok.isNot(tok::eof)) {
       // Each iteration of this loop reads one member-declaration.
       ParseCXXClassMemberDeclarationWithPragmas(
-          CurAS, AccessAttrs, static_cast<DeclSpec::TST>(TagType), TagDecl);
+          CurAS, static_cast<DeclSpec::TST>(TagType), TagDecl);
       MaybeDestroyTemplateIds();
     }
     T.consumeClose();
@@ -4729,9 +4718,8 @@
   Attrs.Range = SourceRange(StartLoc, EndLoc);
 }
 
-void Parser::ParseMicrosoftIfExistsClassDeclaration(
-    DeclSpec::TST TagType, ParsedAttributes &AccessAttrs,
-    AccessSpecifier &CurAS) {
+void Parser::ParseMicrosoftIfExistsClassDeclaration(DeclSpec::TST TagType,
+                                                    AccessSpecifier &CurAS) {
   IfExistsCondition Result;
   if (ParseMicrosoftIfExistsCondition(Result))
     return;
@@ -4761,8 +4749,7 @@
   while (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
     // __if_exists, __if_not_exists can nest.
     if (Tok.isOneOf(tok::kw___if_exists, tok::kw___if_not_exists)) {
-      ParseMicrosoftIfExistsClassDeclaration(TagType,
-                                             AccessAttrs, CurAS);
+      ParseMicrosoftIfExistsClassDeclaration(TagType, CurAS);
       continue;
     }
 
@@ -4788,7 +4775,7 @@
     }
 
     // Parse all the comma separated declarators.
-    ParseCXXClassMemberDeclaration(CurAS, AccessAttrs);
+    ParseCXXClassMemberDeclaration(CurAS);
   }
 
   Braces.consumeClose();
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -1762,7 +1762,7 @@
   case tok::kw_template:
   case tok::kw_export:
     ProhibitAttributes(Attrs);
-    SingleDecl = ParseDeclarationStartingWithTemplate(Context, DeclEnd, Attrs);
+    SingleDecl = ParseDeclarationStartingWithTemplate(Context, DeclEnd);
     break;
   case tok::kw_inline:
     // Could be the start of an inline namespace. Allowed as an ext in C++03.
@@ -4405,8 +4405,7 @@
     if (Tok.isOneOf(tok::annot_pragma_openmp, tok::annot_attr_openmp)) {
       // Result can be ignored, because it must be always empty.
       AccessSpecifier AS = AS_none;
-      ParsedAttributes Attrs(AttrFactory);
-      (void)ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, Attrs);
+      (void)ParseOpenMPDeclarativeDirectiveWithExtDecl(AS);
       continue;
     }
 
Index: clang/lib/Parse/ParseCXXInlineMethods.cpp
===================================================================
--- clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -21,10 +21,11 @@
 /// ParseCXXInlineMethodDef - We parsed and verified that the specified
 /// Declarator is a well formed C++ inline method definition. Now lex its body
 /// and store its tokens for parsing after the C++ class is complete.
-NamedDecl *Parser::ParseCXXInlineMethodDef(
-    AccessSpecifier AS, const ParsedAttributesView &AccessAttrs,
-    ParsingDeclarator &D, const ParsedTemplateInfo &TemplateInfo,
-    const VirtSpecifiers &VS, SourceLocation PureSpecLoc) {
+NamedDecl *
+Parser::ParseCXXInlineMethodDef(AccessSpecifier AS, ParsingDeclarator &D,
+                                const ParsedTemplateInfo &TemplateInfo,
+                                const VirtSpecifiers &VS,
+                                SourceLocation PureSpecLoc) {
   assert(D.isFunctionDeclarator() && "This isn't a function declarator!");
   assert(Tok.isOneOf(tok::l_brace, tok::colon, tok::kw_try, tok::equal) &&
          "Current token not a '{', ':', '=', or 'try'!");
@@ -43,7 +44,6 @@
                                            TemplateParams, nullptr,
                                            VS, ICIS_NoInit);
     if (FnD) {
-      Actions.ProcessDeclAttributeList(getCurScope(), FnD, AccessAttrs);
       if (PureSpecLoc.isValid())
         Actions.ActOnPureSpecifier(FnD, PureSpecLoc);
     }
@@ -795,8 +795,7 @@
   case tok::annot_attr_openmp:
   case tok::annot_pragma_openmp: {
     AccessSpecifier AS = LP.getAccessSpecifier();
-    ParsedAttributes Attrs(AttrFactory);
-    (void)ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, Attrs);
+    (void)ParseOpenMPDeclarativeDirectiveWithExtDecl(AS);
     break;
   }
   default:
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -1544,7 +1544,6 @@
   };
 
   NamedDecl *ParseCXXInlineMethodDef(AccessSpecifier AS,
-                                     const ParsedAttributesView &AccessAttrs,
                                      ParsingDeclarator &D,
                                      const ParsedTemplateInfo &TemplateInfo,
                                      const VirtSpecifiers &VS,
@@ -2136,7 +2135,6 @@
   void ParseMicrosoftIfExistsStatement(StmtVector &Stmts);
   void ParseMicrosoftIfExistsExternalDeclaration();
   void ParseMicrosoftIfExistsClassDeclaration(DeclSpec::TST TagType,
-                                              ParsedAttributes &AccessAttrs,
                                               AccessSpecifier &CurAS);
   bool ParseMicrosoftIfExistsBraceInitializer(ExprVector &InitExprs,
                                               bool &InitExprsOk);
@@ -3080,12 +3078,11 @@
   void MaybeParseAndDiagnoseDeclSpecAfterCXX11VirtSpecifierSeq(Declarator &D,
                                                                VirtSpecifiers &VS);
   DeclGroupPtrTy ParseCXXClassMemberDeclaration(
-      AccessSpecifier AS, ParsedAttributes &Attr,
+      AccessSpecifier AS,
       const ParsedTemplateInfo &TemplateInfo = ParsedTemplateInfo(),
       ParsingDeclRAIIObject *DiagsFromTParams = nullptr);
   DeclGroupPtrTy
   ParseCXXClassMemberDeclarationWithPragmas(AccessSpecifier &AS,
-                                            ParsedAttributes &AccessAttrs,
                                             DeclSpec::TST TagType, Decl *Tag);
   void ParseConstructorInitializer(Decl *ConstructorDecl);
   MemInitResult ParseMemInitializer(Decl *ConstructorDecl);
@@ -3198,7 +3195,7 @@
 
   /// Parses declarative OpenMP directives.
   DeclGroupPtrTy ParseOpenMPDeclarativeDirectiveWithExtDecl(
-      AccessSpecifier &AS, ParsedAttributes &Attrs, bool Delayed = false,
+      AccessSpecifier &AS, bool Delayed = false,
       DeclSpec::TST TagType = DeclSpec::TST_unspecified,
       Decl *TagDecl = nullptr);
   /// Parse 'omp declare reduction' construct.
@@ -3366,16 +3363,14 @@
   // C++ 14.1: Template Parameters [temp.param]
   Decl *ParseDeclarationStartingWithTemplate(DeclaratorContext Context,
                                              SourceLocation &DeclEnd,
-                                             ParsedAttributes &AccessAttrs,
                                              AccessSpecifier AS = AS_none);
   Decl *ParseTemplateDeclarationOrSpecialization(DeclaratorContext Context,
                                                  SourceLocation &DeclEnd,
-                                                 ParsedAttributes &AccessAttrs,
                                                  AccessSpecifier AS);
   Decl *ParseSingleDeclarationAfterTemplate(
       DeclaratorContext Context, const ParsedTemplateInfo &TemplateInfo,
       ParsingDeclRAIIObject &DiagsFromParams, SourceLocation &DeclEnd,
-      ParsedAttributes &AccessAttrs, AccessSpecifier AS = AS_none);
+      AccessSpecifier AS = AS_none);
   bool ParseTemplateParameters(MultiParseScope &TemplateScopes, unsigned Depth,
                                SmallVectorImpl<NamedDecl *> &TemplateParams,
                                SourceLocation &LAngleLoc,
@@ -3424,7 +3419,6 @@
                                    SourceLocation ExternLoc,
                                    SourceLocation TemplateLoc,
                                    SourceLocation &DeclEnd,
-                                   ParsedAttributes &AccessAttrs,
                                    AccessSpecifier AS = AS_none);
   // C++2a: Template, concept definition [temp]
   Decl *
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to