[clang] [Clang][Parse] Diagnose member template declarations with multiple declarators (PR #78243)

Krystian Stasiowski via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 16 10:27:39 PST 2024


https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/78243

>From b5d814abf6241d3e9cdfcc64109c33c56a896172 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 12 Jan 2024 13:45:15 -0500
Subject: [PATCH 1/2] [Clang][Parse] Diagnose member template declarations with
 multiple declarators

---
 clang/docs/ReleaseNotes.rst                 | 1 +
 clang/lib/Parse/ParseDeclCXX.cpp            | 9 +++++++++
 clang/test/CXX/temp/p3.cpp                  | 6 ++++++
 clang/test/OpenMP/declare_simd_messages.cpp | 3 +--
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ea57769a4a5795..54debc4d40e945 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -555,6 +555,7 @@ Improvements to Clang's diagnostics
 - Clang now diagnoses unexpanded packs within the template argument lists of function template specializations.
 - Clang now diagnoses attempts to bind a bitfield to an NTTP of a reference type as erroneous
   converted constant expression and not as a reference to subobject.
+- Clang now diagnoses member template declarations with multiple declarators.
 
 
 Improvements to Clang's time-trace
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 5576be9e717a9b..03a7ac3205d296 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -3167,6 +3167,15 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
 
     DeclaratorInfo.complete(ThisDecl);
 
+    if (TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate) {
+      if (Tok.is(tok::comma)) {
+        Diag(Tok, diag::err_multiple_template_declarators)
+            << (int)TemplateInfo.Kind;
+        SkipUntil(tok::semi, StopBeforeMatch);
+      }
+      break;
+    }
+
     // If we don't have a comma, it is either the end of the list (a ';')
     // or an error, bail out.
     SourceLocation CommaLoc;
diff --git a/clang/test/CXX/temp/p3.cpp b/clang/test/CXX/temp/p3.cpp
index b708c613d352d2..9e561d0b9a83b2 100644
--- a/clang/test/CXX/temp/p3.cpp
+++ b/clang/test/CXX/temp/p3.cpp
@@ -15,3 +15,9 @@ template<typename T> struct B { } f(); // expected-error {{expected ';' after st
 template<typename T> struct C { } // expected-error {{expected ';' after struct}}
 
 A<int> c;
+
+struct D {
+  template<typename T> static const int x = 0, f(); // expected-error {{can only declare a single entity}}
+
+  template<typename T> static const int g(), y = 0; // expected-error {{can only declare a single entity}}
+};
diff --git a/clang/test/OpenMP/declare_simd_messages.cpp b/clang/test/OpenMP/declare_simd_messages.cpp
index dd24322694b69f..fea045400e1faf 100644
--- a/clang/test/OpenMP/declare_simd_messages.cpp
+++ b/clang/test/OpenMP/declare_simd_messages.cpp
@@ -33,10 +33,9 @@ int main();
 int main();
 
 struct A {
-// expected-error at +1 {{function declaration is expected after 'declare simd' directive}}
   #pragma omp declare simd
   template<typename T>
-  T infunc1(T a), infunc2(T a);
+  T infunc1(T a);
 };
 
 // expected-error at +1 {{single declaration is expected after 'declare simd' directive}}

>From 00d26bb294d1a37275e4c781f81d354e26a17100 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Tue, 16 Jan 2024 13:27:27 -0500
Subject: [PATCH 2/2] [WIP] parse multiple declarators in template-declarations

---
 clang/include/clang/Parse/Parser.h |   3 +-
 clang/lib/Parse/ParseDecl.cpp      |  72 ++++++++++++++--
 clang/lib/Parse/ParseDeclCXX.cpp   |  14 ++--
 clang/lib/Parse/ParseTemplate.cpp  | 130 +++--------------------------
 clang/lib/Parse/Parser.cpp         |   4 +-
 5 files changed, 88 insertions(+), 135 deletions(-)

diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index e50a4d05b45991..4cbdb3f8f74340 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -2419,6 +2419,7 @@ class Parser : public CodeCompletionHandler {
   bool MightBeDeclarator(DeclaratorContext Context);
   DeclGroupPtrTy ParseDeclGroup(ParsingDeclSpec &DS, DeclaratorContext Context,
                                 ParsedAttributes &Attrs,
+                                ParsedTemplateInfo &TemplateInfo,
                                 SourceLocation *DeclEnd = nullptr,
                                 ForRangeInit *FRI = nullptr);
   Decl *ParseDeclarationAfterDeclarator(Declarator &D,
@@ -3595,7 +3596,7 @@ class Parser : public CodeCompletionHandler {
                                                  ParsedAttributes &AccessAttrs,
                                                  AccessSpecifier AS);
   Decl *ParseSingleDeclarationAfterTemplate(
-      DeclaratorContext Context, const ParsedTemplateInfo &TemplateInfo,
+      DeclaratorContext Context, ParsedTemplateInfo &TemplateInfo,
       ParsingDeclRAIIObject &DiagsFromParams, SourceLocation &DeclEnd,
       ParsedAttributes &AccessAttrs, AccessSpecifier AS = AS_none);
   bool ParseTemplateParameters(MultiParseScope &TemplateScopes, unsigned Depth,
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 8d856cc2cf8313..9a9dfc790e81f0 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2027,7 +2027,8 @@ Parser::DeclGroupPtrTy Parser::ParseSimpleDeclaration(
   if (DeclSpecStart)
     DS.SetRangeStart(*DeclSpecStart);
 
-  return ParseDeclGroup(DS, Context, DeclAttrs, &DeclEnd, FRI);
+  ParsedTemplateInfo TemplateInfo;
+  return ParseDeclGroup(DS, Context, DeclAttrs, TemplateInfo, &DeclEnd, FRI);
 }
 
 /// Returns true if this might be the start of a declarator, or a common typo
@@ -2184,6 +2185,7 @@ void Parser::SkipMalformedDecl() {
 Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
                                               DeclaratorContext Context,
                                               ParsedAttributes &Attrs,
+                                              ParsedTemplateInfo &TemplateInfo,
                                               SourceLocation *DeclEnd,
                                               ForRangeInit *FRI) {
   // Parse the first declarator.
@@ -2193,8 +2195,29 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
   ParsedAttributes LocalAttrs(AttrFactory);
   LocalAttrs.takeAllFrom(Attrs);
   ParsingDeclarator D(*this, DS, LocalAttrs, Context);
+  if (TemplateInfo.TemplateParams)
+    D.setTemplateParameterLists(*TemplateInfo.TemplateParams);
+
+  // Turn off usual access checking for template specializations and
+  // instantiations.
+  // C++20 [temp.spec] 13.9/6.
+  // This disables the access checking rules for function template explicit
+  // instantiation and explicit specialization:
+  // - parameter-list;
+  // - template-argument-list;
+  // - noexcept-specifier;
+  // - dynamic-exception-specifications (deprecated in C++11, removed since
+  //   C++17).
+  bool IsTemplateSpecOrInst =
+      (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation ||
+       TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
+  SuppressAccessChecks SAC(*this, IsTemplateSpecOrInst);
+
   ParseDeclarator(D);
 
+  if (IsTemplateSpecOrInst)
+    SAC.done();
+
   // Bail out if the first declarator didn't seem well-formed.
   if (!D.hasName() && !D.mayOmitIdentifier()) {
     SkipMalformedDecl();
@@ -2204,8 +2227,14 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
   if (getLangOpts().HLSL)
     MaybeParseHLSLSemantics(D);
 
-  if (Tok.is(tok::kw_requires))
+  if (Tok.is(tok::kw_requires)) {
+    CXXScopeSpec &ScopeSpec = D.getCXXScopeSpec();
+    DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
+    if (ScopeSpec.isValid() &&
+      Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
+    DeclScopeObj.EnterDeclaratorScope();
     ParseTrailingRequiresClause(D);
+  }
 
   // Save late-parsed attributes for now; they need to be parsed in the
   // appropriate function scope after the function Decl has been constructed.
@@ -2269,7 +2298,33 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
             DS.ClearStorageClassSpecs();
           }
 
-          Decl *TheDecl = ParseFunctionDefinition(D, ParsedTemplateInfo(),
+          if (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation) {
+            if (D.getName().getKind() !=
+                UnqualifiedIdKind::IK_TemplateId) {
+              // If the declarator-id is not a template-id, issue a diagnostic and
+              // recover by ignoring the 'template' keyword.
+              Diag(Tok, diag::err_template_defn_explicit_instantiation) << 0;
+              TemplateInfo = ParsedTemplateInfo();
+            } else {
+              SourceLocation LAngleLoc
+                = PP.getLocForEndOfToken(TemplateInfo.TemplateLoc);
+              Diag(D.getIdentifierLoc(),
+                   diag::err_explicit_instantiation_with_definition)
+                  << SourceRange(TemplateInfo.TemplateLoc)
+                  << FixItHint::CreateInsertion(LAngleLoc, "<>");
+
+              // Recover as if it were an explicit specialization.
+              TemplateParameterLists FakedParamLists;
+              FakedParamLists.push_back(Actions.ActOnTemplateParameterList(
+                  0, SourceLocation(), TemplateInfo.TemplateLoc, LAngleLoc,
+                  std::nullopt, LAngleLoc, nullptr));
+              TemplateInfo = ParsedTemplateInfo(&FakedParamLists,
+                                                /*isSpecialization=*/true,
+                                                /*lastParameterListWasEmpty=*/true);
+            }
+          }
+
+          Decl *TheDecl = ParseFunctionDefinition(D, TemplateInfo,
                                                   &LateParsedAttrs);
           return Actions.ConvertDeclToDeclGroup(TheDecl);
         }
@@ -2335,7 +2390,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
 
   SmallVector<Decl *, 8> DeclsInGroup;
   Decl *FirstDecl = ParseDeclarationAfterDeclaratorAndAttributes(
-      D, ParsedTemplateInfo(), FRI);
+      D, TemplateInfo, FRI);
   if (LateParsedAttrs.size() > 0)
     ParseLexedAttributeList(LateParsedAttrs, FirstDecl, true, false);
   D.complete(FirstDecl);
@@ -2344,6 +2399,13 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
 
   bool ExpectSemi = Context != DeclaratorContext::ForInit;
 
+  if (TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate &&
+      Tok.is(tok::comma)) {
+    Diag(Tok, diag::err_multiple_template_declarators)
+      << (int)TemplateInfo.Kind;
+    // SkipUntil(tok::semi);
+  }
+
   // If we don't have a comma, it is either the end of the list (a ';') or an
   // error, bail out.
   SourceLocation CommaLoc;
@@ -2387,7 +2449,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
       //        declarator requires-clause
       if (Tok.is(tok::kw_requires))
         ParseTrailingRequiresClause(D);
-      Decl *ThisDecl = ParseDeclarationAfterDeclarator(D);
+      Decl *ThisDecl = ParseDeclarationAfterDeclarator(D, TemplateInfo);
       D.complete(ThisDecl);
       if (ThisDecl)
         DeclsInGroup.push_back(ThisDecl);
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 03a7ac3205d296..88e900c5e2f20d 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -3024,7 +3024,7 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
   // member-declarator-list:
   //   member-declarator
   //   member-declarator-list ',' member-declarator
-
+  bool PastFirst = false;
   while (true) {
     InClassInitStyle HasInClassInit = ICIS_NoInit;
     bool HasStaticInitializer = false;
@@ -3167,14 +3167,12 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
 
     DeclaratorInfo.complete(ThisDecl);
 
-    if (TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate) {
-      if (Tok.is(tok::comma)) {
-        Diag(Tok, diag::err_multiple_template_declarators)
-            << (int)TemplateInfo.Kind;
-        SkipUntil(tok::semi, StopBeforeMatch);
-      }
-      break;
+    if (TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate &&
+        !PastFirst && Tok.is(tok::comma)) {
+      Diag(Tok, diag::err_multiple_template_declarators)
+           << (int)TemplateInfo.Kind;
     }
+    PastFirst = true;
 
     // If we don't have a comma, it is either the end of the list (a ';')
     // or an error, bail out.
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index 64fe4d50bba27b..05e4434f0c6d90 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -168,9 +168,10 @@ Decl *Parser::ParseTemplateDeclarationOrSpecialization(
                            LastParamListWasEmpty),
         DeclEnd);
 
+  ParsedTemplateInfo TemplateInfo(&ParamLists, isSpecialization, LastParamListWasEmpty);
   return ParseSingleDeclarationAfterTemplate(
       Context,
-      ParsedTemplateInfo(&ParamLists, isSpecialization, LastParamListWasEmpty),
+      TemplateInfo,
       ParsingTemplateParams, DeclEnd, AccessAttrs, AS);
 }
 
@@ -185,7 +186,7 @@ Decl *Parser::ParseTemplateDeclarationOrSpecialization(
 ///
 /// \returns the new declaration.
 Decl *Parser::ParseSingleDeclarationAfterTemplate(
-    DeclaratorContext Context, const ParsedTemplateInfo &TemplateInfo,
+    DeclaratorContext Context, ParsedTemplateInfo &TemplateInfo,
     ParsingDeclRAIIObject &DiagsFromTParams, SourceLocation &DeclEnd,
     ParsedAttributes &AccessAttrs, AccessSpecifier AS) {
   assert(TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate &&
@@ -262,123 +263,13 @@ Decl *Parser::ParseSingleDeclarationAfterTemplate(
   if (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation)
     ProhibitAttributes(prefixAttrs);
 
-  // Parse the declarator.
-  ParsingDeclarator DeclaratorInfo(*this, DS, prefixAttrs,
-                                   (DeclaratorContext)Context);
-  if (TemplateInfo.TemplateParams)
-    DeclaratorInfo.setTemplateParameterLists(*TemplateInfo.TemplateParams);
-
-  // Turn off usual access checking for template specializations and
-  // instantiations.
-  // C++20 [temp.spec] 13.9/6.
-  // This disables the access checking rules for function template explicit
-  // instantiation and explicit specialization:
-  // - parameter-list;
-  // - template-argument-list;
-  // - noexcept-specifier;
-  // - dynamic-exception-specifications (deprecated in C++11, removed since
-  //   C++17).
-  bool IsTemplateSpecOrInst =
-      (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation ||
-       TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
-  SuppressAccessChecks SAC(*this, IsTemplateSpecOrInst);
-
-  ParseDeclarator(DeclaratorInfo);
-
-  if (IsTemplateSpecOrInst)
-    SAC.done();
-
-  // Error parsing the declarator?
-  if (!DeclaratorInfo.hasName()) {
-    SkipMalformedDecl();
+  auto DeclGroupPtr = ParseDeclGroup(DS, Context,
+                                     prefixAttrs, TemplateInfo,
+                                     &DeclEnd,
+                                     /*FRI=*/nullptr);
+  if (!DeclGroupPtr || !DeclGroupPtr.get().isSingleDecl())
     return nullptr;
-  }
-
-  LateParsedAttrList LateParsedAttrs(true);
-  if (DeclaratorInfo.isFunctionDeclarator()) {
-    if (Tok.is(tok::kw_requires)) {
-      CXXScopeSpec &ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
-      DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
-      if (ScopeSpec.isValid() &&
-          Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
-        DeclScopeObj.EnterDeclaratorScope();
-      ParseTrailingRequiresClause(DeclaratorInfo);
-    }
-
-    MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs);
-  }
-
-  if (DeclaratorInfo.isFunctionDeclarator() &&
-      isStartOfFunctionDefinition(DeclaratorInfo)) {
-
-    // Function definitions are only allowed at file scope and in C++ classes.
-    // The C++ inline method definition case is handled elsewhere, so we only
-    // need to handle the file scope definition case.
-    if (Context != DeclaratorContext::File) {
-      Diag(Tok, diag::err_function_definition_not_allowed);
-      SkipMalformedDecl();
-      return nullptr;
-    }
-
-    if (DS.getStorageClassSpec() == DeclSpec::SCS_typedef) {
-      // Recover by ignoring the 'typedef'. This was probably supposed to be
-      // the 'typename' keyword, which we should have already suggested adding
-      // if it's appropriate.
-      Diag(DS.getStorageClassSpecLoc(), diag::err_function_declared_typedef)
-        << FixItHint::CreateRemoval(DS.getStorageClassSpecLoc());
-      DS.ClearStorageClassSpecs();
-    }
-
-    if (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation) {
-      if (DeclaratorInfo.getName().getKind() !=
-          UnqualifiedIdKind::IK_TemplateId) {
-        // If the declarator-id is not a template-id, issue a diagnostic and
-        // recover by ignoring the 'template' keyword.
-        Diag(Tok, diag::err_template_defn_explicit_instantiation) << 0;
-        return ParseFunctionDefinition(DeclaratorInfo, ParsedTemplateInfo(),
-                                       &LateParsedAttrs);
-      } else {
-        SourceLocation LAngleLoc
-          = PP.getLocForEndOfToken(TemplateInfo.TemplateLoc);
-        Diag(DeclaratorInfo.getIdentifierLoc(),
-             diag::err_explicit_instantiation_with_definition)
-            << SourceRange(TemplateInfo.TemplateLoc)
-            << FixItHint::CreateInsertion(LAngleLoc, "<>");
-
-        // Recover as if it were an explicit specialization.
-        TemplateParameterLists FakedParamLists;
-        FakedParamLists.push_back(Actions.ActOnTemplateParameterList(
-            0, SourceLocation(), TemplateInfo.TemplateLoc, LAngleLoc,
-            std::nullopt, LAngleLoc, nullptr));
-
-        return ParseFunctionDefinition(
-            DeclaratorInfo, ParsedTemplateInfo(&FakedParamLists,
-                                               /*isSpecialization=*/true,
-                                               /*lastParameterListWasEmpty=*/true),
-            &LateParsedAttrs);
-      }
-    }
-    return ParseFunctionDefinition(DeclaratorInfo, TemplateInfo,
-                                   &LateParsedAttrs);
-  }
-
-  // Parse this declaration.
-  Decl *ThisDecl = ParseDeclarationAfterDeclarator(DeclaratorInfo,
-                                                   TemplateInfo);
-
-  if (Tok.is(tok::comma)) {
-    Diag(Tok, diag::err_multiple_template_declarators)
-      << (int)TemplateInfo.Kind;
-    SkipUntil(tok::semi);
-    return ThisDecl;
-  }
-
-  // Eat the semi colon after the declaration.
-  ExpectAndConsumeSemi(diag::err_expected_semi_declaration);
-  if (LateParsedAttrs.size() > 0)
-    ParseLexedAttributeList(LateParsedAttrs, ThisDecl, true, false);
-  DeclaratorInfo.complete(ThisDecl);
-  return ThisDecl;
+  return DeclGroupPtr.get().getSingleDecl();
 }
 
 /// \brief Parse a single declaration that declares a concept.
@@ -1696,8 +1587,9 @@ Decl *Parser::ParseExplicitInstantiation(DeclaratorContext Context,
   ParsingDeclRAIIObject
     ParsingTemplateParams(*this, ParsingDeclRAIIObject::NoParent);
 
+  ParsedTemplateInfo TemplateInfo(ExternLoc, TemplateLoc);
   return ParseSingleDeclarationAfterTemplate(
-      Context, ParsedTemplateInfo(ExternLoc, TemplateLoc),
+      Context, TemplateInfo,
       ParsingTemplateParams, DeclEnd, AccessAttrs, AS);
 }
 
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index b703c2d9b8e04d..5f2eb23a17d81b 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -1235,8 +1235,8 @@ Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal(
     Decl *TheDecl = ParseLinkage(DS, DeclaratorContext::File);
     return Actions.ConvertDeclToDeclGroup(TheDecl);
   }
-
-  return ParseDeclGroup(DS, DeclaratorContext::File, Attrs);
+  ParsedTemplateInfo TemplateInfo;
+  return ParseDeclGroup(DS, DeclaratorContext::File, Attrs, TemplateInfo);
 }
 
 Parser::DeclGroupPtrTy Parser::ParseDeclarationOrFunctionDefinition(



More information about the cfe-commits mailing list