[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-06 Thread Krystian Stasiowski via cfe-commits 

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

>From 39744e6eb218ae519bcce83d90273296de496d94 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski 
Date: Fri, 25 Aug 2023 14:07:32 -0400
Subject: [PATCH] [clang] remove ClassScopeFunctionSpecializationDecl

---
 .../clang-tidy/modernize/UseEmplaceCheck.cpp  |   8 +-
 .../clangd/SemanticHighlighting.cpp   |   9 --
 clang/docs/ReleaseNotes.rst   |   5 +
 clang/include/clang/AST/ASTNodeTraverser.h|  11 +-
 clang/include/clang/AST/Decl.h|  10 +-
 clang/include/clang/AST/DeclTemplate.h| 150 +++---
 clang/include/clang/AST/RecursiveASTVisitor.h |  17 +-
 clang/include/clang/Basic/DeclNodes.td|   1 -
 .../clang/Basic/DiagnosticSemaKinds.td|   8 +-
 clang/include/clang/Sema/Sema.h   |   6 +-
 clang/include/clang/Sema/Template.h   |   2 -
 .../include/clang/Serialization/ASTBitCodes.h |   4 -
 .../clang/Serialization/ASTRecordReader.h |   2 +
 clang/lib/AST/ASTImporter.cpp |  26 ++-
 clang/lib/AST/Decl.cpp|  71 ++---
 clang/lib/AST/DeclBase.cpp|   5 +-
 clang/lib/AST/DeclTemplate.cpp|  13 --
 clang/lib/AST/ODRHash.cpp |   4 +
 clang/lib/CodeGen/CGDecl.cpp  |   1 -
 clang/lib/Index/IndexSymbol.cpp   |   1 -
 clang/lib/Sema/SemaDecl.cpp   | 112 ++---
 clang/lib/Sema/SemaTemplate.cpp   |  14 +-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  | 123 +++---
 clang/lib/Serialization/ASTCommon.cpp |   1 -
 clang/lib/Serialization/ASTReader.cpp |  19 ++-
 clang/lib/Serialization/ASTReaderDecl.cpp |  65 +++-
 clang/lib/Serialization/ASTWriter.cpp |   1 -
 clang/lib/Serialization/ASTWriterDecl.cpp |  42 ++---
 .../Checkers/SmartPtrModeling.cpp |   2 +-
 clang/test/AST/ast-dump-decl.cpp  |   9 +-
 clang/test/CXX/drs/dr7xx.cpp  |   5 +
 .../test/SemaTemplate/instantiate-method.cpp  |  23 ++-
 clang/tools/libclang/CIndex.cpp   |   1 -
 33 files changed, 312 insertions(+), 459 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index 554abcd900e329c..b85dde5644d313f 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -67,11 +67,9 @@ AST_MATCHER_P(CallExpr, hasLastArgument,
 // function had parameters defined (this is useful to check if there is only 
one
 // variadic argument).
 AST_MATCHER(CXXMemberCallExpr, hasSameNumArgsAsDeclNumParams) {
-  if (Node.getMethodDecl()->isFunctionTemplateSpecialization())
-return Node.getNumArgs() == Node.getMethodDecl()
-->getPrimaryTemplate()
-->getTemplatedDecl()
-->getNumParams();
+  if (const FunctionTemplateDecl *Primary =
+  Node.getMethodDecl()->getPrimaryTemplate())
+return Node.getNumArgs() == Primary->getTemplatedDecl()->getNumParams();
 
   return Node.getNumArgs() == Node.getMethodDecl()->getNumParams();
 }
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 45c01634e2e38d1..7649e37e1f96663 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -715,13 +715,6 @@ class CollectExtraHighlightings
 return true;
   }
 
-  bool VisitClassScopeFunctionSpecializationDecl(
-  ClassScopeFunctionSpecializationDecl *D) {
-if (auto *Args = D->getTemplateArgsAsWritten())
-  H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
-return true;
-  }
-
   bool VisitDeclRefExpr(DeclRefExpr *E) {
 H.addAngleBracketTokens(E->getLAngleLoc(), E->getRAngleLoc());
 return true;
@@ -752,8 +745,6 @@ class CollectExtraHighlightings
 }
 if (auto *Args = D->getTemplateSpecializationArgsAsWritten())
   H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
-if (auto *I = D->getDependentSpecializationInfo())
-  H.addAngleBracketTokens(I->getLAngleLoc(), I->getRAngleLoc());
 return true;
   }
 
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d2a435041a1542f..ea737fdb5fdad15 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -71,6 +71,11 @@ C++ Specific Potentially Breaking Changes
   (`#49884 `_), and
   (`#61273 `_)
 
+- The `ClassScopeFunctionSpecializationDecl` AST node has been removed. 
Dependent class scope
+explicit function template specializations now use 
`DependentFunctionTemplateSpecializati

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-06 Thread Erich Keane via cfe-commits 

erichkeane wrote:

If Aaron doesn't get to it, feel free to ping me on Monday.  Its too late on a 
Friday for me to be able to merge this for you, but monday morning would be 
fine for me.

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-06 Thread Aaron Ballman via cfe-commits 

AaronBallman wrote:

Do you need someone to land these changes on your behalf?

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-03 Thread Krystian Stasiowski via cfe-commits 

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

>From d11d546f3190936ba45c57b4825073026d817878 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski 
Date: Fri, 25 Aug 2023 14:07:32 -0400
Subject: [PATCH 1/5] [clang] remove ClassScopeFunctionSpecializationDecl

---
 .../clang-tidy/modernize/UseEmplaceCheck.cpp  |   2 +-
 .../clangd/SemanticHighlighting.cpp   |   9 --
 clang/include/clang/AST/ASTNodeTraverser.h|   7 +-
 clang/include/clang/AST/Decl.h|  10 +-
 clang/include/clang/AST/DeclTemplate.h| 150 +++---
 clang/include/clang/AST/RecursiveASTVisitor.h |  19 ++-
 clang/include/clang/Basic/DeclNodes.td|   1 -
 .../clang/Basic/DiagnosticSemaKinds.td|   8 +-
 clang/include/clang/Sema/Sema.h   |   6 +-
 clang/include/clang/Sema/Template.h   |   2 -
 .../include/clang/Serialization/ASTBitCodes.h |   4 -
 .../clang/Serialization/ASTRecordReader.h |   2 +
 clang/lib/AST/ASTImporter.cpp |  26 ++-
 clang/lib/AST/Decl.cpp|  71 ++---
 clang/lib/AST/DeclBase.cpp|   2 -
 clang/lib/AST/DeclTemplate.cpp|  13 --
 clang/lib/AST/ODRHash.cpp |   4 +
 clang/lib/CodeGen/CGDecl.cpp  |   1 -
 clang/lib/Index/IndexSymbol.cpp   |   1 -
 clang/lib/Sema/SemaDecl.cpp   | 112 ++---
 clang/lib/Sema/SemaTemplate.cpp   |  14 +-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  | 121 +++---
 clang/lib/Serialization/ASTCommon.cpp |   1 -
 clang/lib/Serialization/ASTReader.cpp |  19 ++-
 clang/lib/Serialization/ASTReaderDecl.cpp |  59 ++-
 clang/lib/Serialization/ASTWriter.cpp |   1 -
 clang/lib/Serialization/ASTWriterDecl.cpp |  42 ++---
 .../Checkers/SmartPtrModeling.cpp |   2 +-
 clang/test/AST/ast-dump-decl.cpp  |   9 +-
 clang/test/CXX/drs/dr7xx.cpp  |   5 +
 .../test/SemaTemplate/instantiate-method.cpp  |   7 +-
 clang/tools/libclang/CIndex.cpp   |   1 -
 32 files changed, 290 insertions(+), 441 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index 554abcd900e329c..06ae1f4e257528f 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -67,7 +67,7 @@ AST_MATCHER_P(CallExpr, hasLastArgument,
 // function had parameters defined (this is useful to check if there is only 
one
 // variadic argument).
 AST_MATCHER(CXXMemberCallExpr, hasSameNumArgsAsDeclNumParams) {
-  if (Node.getMethodDecl()->isFunctionTemplateSpecialization())
+  if (Node.getMethodDecl()->getPrimaryTemplate())
 return Node.getNumArgs() == Node.getMethodDecl()
 ->getPrimaryTemplate()
 ->getTemplatedDecl()
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 45c01634e2e38d1..7649e37e1f96663 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -715,13 +715,6 @@ class CollectExtraHighlightings
 return true;
   }
 
-  bool VisitClassScopeFunctionSpecializationDecl(
-  ClassScopeFunctionSpecializationDecl *D) {
-if (auto *Args = D->getTemplateArgsAsWritten())
-  H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
-return true;
-  }
-
   bool VisitDeclRefExpr(DeclRefExpr *E) {
 H.addAngleBracketTokens(E->getLAngleLoc(), E->getRAngleLoc());
 return true;
@@ -752,8 +745,6 @@ class CollectExtraHighlightings
 }
 if (auto *Args = D->getTemplateSpecializationArgsAsWritten())
   H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
-if (auto *I = D->getDependentSpecializationInfo())
-  H.addAngleBracketTokens(I->getLAngleLoc(), I->getRAngleLoc());
 return true;
   }
 
diff --git a/clang/include/clang/AST/ASTNodeTraverser.h 
b/clang/include/clang/AST/ASTNodeTraverser.h
index 1151a756ff377b6..c3d233d3ab4a259 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -428,6 +428,8 @@ class ASTNodeTraverser
   void VisitFunctionDecl(const FunctionDecl *D) {
 if (const auto *FTSI = D->getTemplateSpecializationInfo())
   dumpTemplateArgumentList(*FTSI->TemplateArguments);
+else if (const auto *DFTSI = D->getDependentSpecializationInfo())
+  dumpASTTemplateArgumentListInfo(DFTSI->TemplateArgumentsAsWritten);
 
 if (D->param_begin())
   for (const auto *Parameter : D->parameters())
@@ -578,11 +580,6 @@ class ASTNodeTraverser
 dumpTemplateParameters(D->getTemplateParameters());
   }
 
-  void VisitClassScopeFunctionSpecializationDecl(
-  const ClassScopeFunctionSpe

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-02 Thread Aaron Ballman via cfe-commits 

https://github.com/AaronBallman approved this pull request.

LGTM aside from a minor suggestion with the release note.

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-02 Thread Aaron Ballman via cfe-commits 

https://github.com/AaronBallman edited 
https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-02 Thread Aaron Ballman via cfe-commits 

AaronBallman wrote:

CC @llvm/clang-vendors for awareness of the AST changing to remove a node.

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-02 Thread Krystian Stasiowski via cfe-commits 

sdkrystian wrote:

@AaronBallman 
> Can you be sure to file an issue or add a test case + FIXME comment so we 
> don't lose track of the needed follow-up work, or are you planning to do that 
> work immediately after this lands?

I will address that once this gets merged

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-02 Thread Krystian Stasiowski via cfe-commits 


@@ -752,8 +745,6 @@ class CollectExtraHighlightings
 }
 if (auto *Args = D->getTemplateSpecializationArgsAsWritten())
   H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
-if (auto *I = D->getDependentSpecializationInfo())
-  H.addAngleBracketTokens(I->getLAngleLoc(), I->getRAngleLoc());

sdkrystian wrote:

@AaronBallman With this patch we store the template arguments as written in 
`DependentFunctionTemplateSpecializationInfo::TemplateArgsAsWritten`, and 
`FunctionDecl::getTemplateSpecializationArgsAsWritten` is updated to return the 
stored `ASTTemplateArgumentListInfo`. Therefore, the two lines above handle the 
arguments list for dependent function template specializations.

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-02 Thread Aaron Ballman via cfe-commits 


@@ -10442,32 +10441,54 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator 
&D, DeclContext *DC,
 if (getLangOpts().CUDA && !isFunctionTemplateSpecialization)
   maybeAddCUDAHostDeviceAttrs(NewFD, Previous);
 
-// If it's a friend (and only if it's a friend), it's possible
-// that either the specialized function type or the specialized
-// template is dependent, and therefore matching will fail.  In
-// this case, don't check the specialization yet.
-if (isFunctionTemplateSpecialization && isFriend &&
-(NewFD->getType()->isDependentType() || DC->isDependentContext() ||
- 
TemplateSpecializationType::anyInstantiationDependentTemplateArguments(
- TemplateArgs.arguments( {
-  assert(HasExplicitTemplateArgs &&
- "friend function specialization without template args");
-  if (CheckDependentFunctionTemplateSpecialization(NewFD, TemplateArgs,
-   Previous))
-NewFD->setInvalidDecl();
-} else if (isFunctionTemplateSpecialization) {
-  if (CurContext->isDependentContext() && CurContext->isRecord()
-  && !isFriend) {
-isDependentClassScopeExplicitSpecialization = true;
-  } else if (!NewFD->isInvalidDecl() &&
- CheckFunctionTemplateSpecialization(
- NewFD, (HasExplicitTemplateArgs ? &TemplateArgs : 
nullptr),
- Previous))
-NewFD->setInvalidDecl();
+// Handle explict specializations of function templates
+// and friend function declarations with an explicit
+// template argument list.
+if (isFunctionTemplateSpecialization) {
+  bool isDependentSpecialization = false;
+  if (isFriend) {
+// For friend function specializations, this is a dependent
+// specialization if its semantic context is dependent, its
+// type is dependent, or if its template-id is dependent.
+isDependentSpecialization =
+DC->isDependentContext() || NewFD->getType()->isDependentType() ||
+(HasExplicitTemplateArgs &&
+ TemplateSpecializationType::
+ anyInstantiationDependentTemplateArguments(
+ TemplateArgs.arguments()));
+assert(!isDependentSpecialization ||
+   (HasExplicitTemplateArgs == isDependentSpecialization) &&
+   "dependent friend function specialization without template "
+   "args");
+  } else {
+// For class-scope explicit specializations of function templates,
+// if the lexical context is dependent, then the specialization
+// is dependent.
+isDependentSpecialization =
+CurContext->isRecord() && CurContext->isDependentContext();
+  }
+
+  TemplateArgumentListInfo *ExplicitTemplateArgs =
+  HasExplicitTemplateArgs ? &TemplateArgs : nullptr;
+  if (isDependentSpecialization) {
+// If it's a dependent specialization, it may not be possible
+// to determine the primary template (for explicit specializations)
+// or befriended declaration (for friends) until the enclosing
+// template is instantiated. In such cases, we store the declarations
+// found by name lookup and defer resolution until instantiation.
+if (CheckDependentFunctionTemplateSpecialization(
+NewFD, ExplicitTemplateArgs, Previous))
+  NewFD->setInvalidDecl();
+  } else if (!NewFD->isInvalidDecl()) {
+if (CheckFunctionTemplateSpecialization(NewFD, ExplicitTemplateArgs,
+Previous))
+  NewFD->setInvalidDecl();
+  }
 
   // C++ [dcl.stc]p1:
   //   A storage-class-specifier shall not be specified in an explicit
   //   specialization (14.7.3)
+  // FIXME: We should be checking this for dependent specializations.

AaronBallman wrote:

Can you be sure to file an issue or add a test case + FIXME comment so we don't 
lose track of the needed follow-up work, or are you planning to do that work 
immediately after this lands?

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-02 Thread Aaron Ballman via cfe-commits 


@@ -752,8 +745,6 @@ class CollectExtraHighlightings
 }
 if (auto *Args = D->getTemplateSpecializationArgsAsWritten())
   H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
-if (auto *I = D->getDependentSpecializationInfo())
-  H.addAngleBracketTokens(I->getLAngleLoc(), I->getRAngleLoc());

AaronBallman wrote:

Can you explain why this was removed?

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-02 Thread Aaron Ballman via cfe-commits 

https://github.com/AaronBallman commented:

The changes should definitely have a release note so that downstreams can be 
aware of the changes. This isn't really a potentially breaking change, but we 
should alert clang-vendors in case a downstream actually cares.

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-10-02 Thread Aaron Ballman via cfe-commits 


@@ -9354,13 +9353,14 @@ 
Sema::CheckDependentFunctionTemplateSpecialization(FunctionDecl *FD,
   }
   F.done();
 
+  bool isFriend = FD->getFriendObjectKind() != Decl::FOK_None;

AaronBallman wrote:

```suggestion
  bool IsFriend = FD->getFriendObjectKind() != Decl::FOK_None;
```
nit for naming style.

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-21 Thread Krystian Stasiowski via cfe-commits 


@@ -1016,21 +1003,20 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) 
{
   }
   case FunctionDecl::TK_DependentFunctionTemplateSpecialization: {
 // Templates.
-UnresolvedSet<8> TemplDecls;
-unsigned NumTemplates = Record.readInt();
-while (NumTemplates--)
-  TemplDecls.addDecl(readDeclAs());
+UnresolvedSet<8> Candidates;
+unsigned NumCandidates = Record.readInt();

sdkrystian wrote:

@shafik  The "convention" here seems to be to use the type of the member being 
deserialized (see lines 1082 & 1122). The `UnresolvedSet` is only used to pass 
the candidates to `DependentFunctionTemplateSpecializationInfo::Create`; the 
actual candidates are stored as trailing objects & the number of candidates is 
stored in `DependentFunctionTemplateSpecializationInfo::NumCandidates` (which 
has the type `unsigned`).

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Krystian Stasiowski via cfe-commits 


@@ -650,17 +642,16 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) {
 DependentFunctionTemplateSpecializationInfo *
   DFTSInfo = D->getDependentSpecializationInfo();
 
-// Templates.
-Record.push_back(DFTSInfo->getNumTemplates());
-for (int i=0, e = DFTSInfo->getNumTemplates(); i != e; ++i)
-  Record.AddDeclRef(DFTSInfo->getTemplate(i));
+// Candidates.
+Record.push_back(DFTSInfo->getCandidates().size());

sdkrystian wrote:

`DependentFunctionTemplateSpecializationInfo` stores the number of candidates 
in an `unsigned` member, so it should be fine. 

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Shafik Yaghmour via cfe-commits 


@@ -4086,7 +4073,7 @@ FunctionDecl 
*Sema::SubstSpaceshipAsEqualEqual(CXXRecordDecl *RD,
   Decl *R;
   if (auto *MD = dyn_cast(Spaceship)) {
 R = Instantiator.VisitCXXMethodDecl(
-MD, nullptr, std::nullopt,
+MD, nullptr,

shafik wrote:

```suggestion
MD, /*TemplateParams=*/nullptr,
```

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Shafik Yaghmour via cfe-commits 


@@ -650,17 +642,16 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) {
 DependentFunctionTemplateSpecializationInfo *
   DFTSInfo = D->getDependentSpecializationInfo();
 
-// Templates.
-Record.push_back(DFTSInfo->getNumTemplates());
-for (int i=0, e = DFTSInfo->getNumTemplates(); i != e; ++i)
-  Record.AddDeclRef(DFTSInfo->getTemplate(i));
+// Candidates.
+Record.push_back(DFTSInfo->getCandidates().size());

shafik wrote:

`getNumTemplates()` returns `unsigned` while `size()` AFAICT returns `size_t` 
which is not the same type. Is this ok?

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Shafik Yaghmour via cfe-commits 

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Krystian Stasiowski via cfe-commits 


@@ -2154,6 +2144,15 @@ bool 
RecursiveASTVisitor::TraverseFunctionHelper(FunctionDecl *D) {
   TALI->NumTemplateArgs));
   }
 }
+// FIXME: Do we want to traverse the explicit template arguments for

sdkrystian wrote:

I originally added this comment because it changes behavior (i.e. explicit 
template arguments for dependent friends were not previously traversed, but are 
traversed with this change). Upon a second examination, I don't think the 
comment is necessary, and the correct behavior is to traverse the explicit 
arguments (as we would traverse them for a non-dependent specialization).

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Krystian Stasiowski via cfe-commits 


@@ -10442,32 +10441,54 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator 
&D, DeclContext *DC,
 if (getLangOpts().CUDA && !isFunctionTemplateSpecialization)
   maybeAddCUDAHostDeviceAttrs(NewFD, Previous);
 
-// If it's a friend (and only if it's a friend), it's possible
-// that either the specialized function type or the specialized
-// template is dependent, and therefore matching will fail.  In
-// this case, don't check the specialization yet.
-if (isFunctionTemplateSpecialization && isFriend &&
-(NewFD->getType()->isDependentType() || DC->isDependentContext() ||
- 
TemplateSpecializationType::anyInstantiationDependentTemplateArguments(
- TemplateArgs.arguments( {
-  assert(HasExplicitTemplateArgs &&
- "friend function specialization without template args");
-  if (CheckDependentFunctionTemplateSpecialization(NewFD, TemplateArgs,
-   Previous))
-NewFD->setInvalidDecl();
-} else if (isFunctionTemplateSpecialization) {
-  if (CurContext->isDependentContext() && CurContext->isRecord()
-  && !isFriend) {
-isDependentClassScopeExplicitSpecialization = true;
-  } else if (!NewFD->isInvalidDecl() &&
- CheckFunctionTemplateSpecialization(
- NewFD, (HasExplicitTemplateArgs ? &TemplateArgs : 
nullptr),
- Previous))
-NewFD->setInvalidDecl();
+// Handle explict specializations of function templates
+// and friend function declarations with an explicit
+// template argument list.
+if (isFunctionTemplateSpecialization) {
+  bool isDependentSpecialization = false;
+  if (isFriend) {
+// For friend function specializations, this is a dependent
+// specialization if its semantic context is dependent, its
+// type is dependent, or if its template-id is dependent.
+isDependentSpecialization =
+DC->isDependentContext() || NewFD->getType()->isDependentType() ||
+(HasExplicitTemplateArgs &&
+ TemplateSpecializationType::
+ anyInstantiationDependentTemplateArguments(
+ TemplateArgs.arguments()));
+assert(!isDependentSpecialization ||
+   (HasExplicitTemplateArgs == isDependentSpecialization) &&
+   "dependent friend function specialization without template "
+   "args");
+  } else {
+// For class-scope explicit specializations of function templates,
+// if the lexical context is dependent, then the specialization
+// is dependent.
+isDependentSpecialization =
+CurContext->isRecord() && CurContext->isDependentContext();
+  }
+
+  TemplateArgumentListInfo *ExplicitTemplateArgs =
+  HasExplicitTemplateArgs ? &TemplateArgs : nullptr;
+  if (isDependentSpecialization) {
+// If it's a dependent specialization, it may not be possible
+// to determine the primary template (for explicit specializations)
+// or befriended declaration (for friends) until the enclosing
+// template is instantiated. In such cases, we store the declarations
+// found by name lookup and defer resolution until instantiation.
+if (CheckDependentFunctionTemplateSpecialization(
+NewFD, ExplicitTemplateArgs, Previous))
+  NewFD->setInvalidDecl();
+  } else if (!NewFD->isInvalidDecl()) {
+if (CheckFunctionTemplateSpecialization(NewFD, ExplicitTemplateArgs,
+Previous))
+  NewFD->setInvalidDecl();
+  }
 
   // C++ [dcl.stc]p1:
   //   A storage-class-specifier shall not be specified in an explicit
   //   specialization (14.7.3)
+  // FIXME: We should be checking this for dependent specializations.

sdkrystian wrote:

I opted to not fix it in this commit to minimize unrelated changes. We _should_ 
be doing this check if `!isFriend` (since we already diagnose storage class 
specifiers on friend declarations elsewhere). Furthermore, the check for 
inconsistent storage class should happen in 
`CheckFunctionTemplateSpecialization` (so we check during the instantiation of 
dependent specializations, once the primary template is known).

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Erich Keane via cfe-commits 


@@ -185,14 +185,11 @@ namespace SameSignatureAfterInstantiation {
 
 namespace PR22040 {
   template  struct Foobar {
-template <> void bazqux(typename T::type) {}  // expected-error 2{{cannot 
be used prior to '::' because it has no members}}

erichkeane wrote:

Can you restore the test here?  We still want to capture that behavior.  
Additionally, despite them not being particularly interesting, please leave the 
ones from the 192-195 as well;

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Erich Keane via cfe-commits 

https://github.com/erichkeane commented:

I didn't spend much time in Serialization, so hopefully someone else can take a 
look, but in general I think I'm warming toward this.

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Erich Keane via cfe-commits 


@@ -2154,6 +2144,15 @@ bool 
RecursiveASTVisitor::TraverseFunctionHelper(FunctionDecl *D) {
   TALI->NumTemplateArgs));
   }
 }
+// FIXME: Do we want to traverse the explicit template arguments for

erichkeane wrote:

I don't have a great idea here... what do we do with other explicit template 
arguments?  My knee-jerk is 'yes'? 

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Erich Keane via cfe-commits 


@@ -428,6 +428,8 @@ class ASTNodeTraverser
   void VisitFunctionDecl(const FunctionDecl *D) {
 if (const auto *FTSI = D->getTemplateSpecializationInfo())
   dumpTemplateArgumentList(*FTSI->TemplateArguments);
+else if (const auto *DFTSI = D->getDependentSpecializationInfo())

erichkeane wrote:

Annoyingly, we're using 'auto' around here, but based on my understanding of 
our coding standard, it isn't permitted here (as the RHS doesn't state the 
type).

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Erich Keane via cfe-commits 


@@ -10442,32 +10441,54 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator 
&D, DeclContext *DC,
 if (getLangOpts().CUDA && !isFunctionTemplateSpecialization)
   maybeAddCUDAHostDeviceAttrs(NewFD, Previous);
 
-// If it's a friend (and only if it's a friend), it's possible
-// that either the specialized function type or the specialized
-// template is dependent, and therefore matching will fail.  In
-// this case, don't check the specialization yet.
-if (isFunctionTemplateSpecialization && isFriend &&
-(NewFD->getType()->isDependentType() || DC->isDependentContext() ||
- 
TemplateSpecializationType::anyInstantiationDependentTemplateArguments(
- TemplateArgs.arguments( {
-  assert(HasExplicitTemplateArgs &&
- "friend function specialization without template args");
-  if (CheckDependentFunctionTemplateSpecialization(NewFD, TemplateArgs,
-   Previous))
-NewFD->setInvalidDecl();
-} else if (isFunctionTemplateSpecialization) {
-  if (CurContext->isDependentContext() && CurContext->isRecord()
-  && !isFriend) {
-isDependentClassScopeExplicitSpecialization = true;
-  } else if (!NewFD->isInvalidDecl() &&
- CheckFunctionTemplateSpecialization(
- NewFD, (HasExplicitTemplateArgs ? &TemplateArgs : 
nullptr),
- Previous))
-NewFD->setInvalidDecl();
+// Handle explict specializations of function templates
+// and friend function declarations with an explicit
+// template argument list.
+if (isFunctionTemplateSpecialization) {
+  bool isDependentSpecialization = false;
+  if (isFriend) {
+// For friend function specializations, this is a dependent
+// specialization if its semantic context is dependent, its
+// type is dependent, or if its template-id is dependent.
+isDependentSpecialization =
+DC->isDependentContext() || NewFD->getType()->isDependentType() ||
+(HasExplicitTemplateArgs &&
+ TemplateSpecializationType::
+ anyInstantiationDependentTemplateArguments(
+ TemplateArgs.arguments()));
+assert(!isDependentSpecialization ||
+   (HasExplicitTemplateArgs == isDependentSpecialization) &&
+   "dependent friend function specialization without template "
+   "args");
+  } else {
+// For class-scope explicit specializations of function templates,
+// if the lexical context is dependent, then the specialization
+// is dependent.
+isDependentSpecialization =
+CurContext->isRecord() && CurContext->isDependentContext();
+  }
+
+  TemplateArgumentListInfo *ExplicitTemplateArgs =
+  HasExplicitTemplateArgs ? &TemplateArgs : nullptr;
+  if (isDependentSpecialization) {
+// If it's a dependent specialization, it may not be possible
+// to determine the primary template (for explicit specializations)
+// or befriended declaration (for friends) until the enclosing
+// template is instantiated. In such cases, we store the declarations
+// found by name lookup and defer resolution until instantiation.
+if (CheckDependentFunctionTemplateSpecialization(
+NewFD, ExplicitTemplateArgs, Previous))
+  NewFD->setInvalidDecl();
+  } else if (!NewFD->isInvalidDecl()) {
+if (CheckFunctionTemplateSpecialization(NewFD, ExplicitTemplateArgs,
+Previous))
+  NewFD->setInvalidDecl();
+  }
 
   // C++ [dcl.stc]p1:
   //   A storage-class-specifier shall not be specified in an explicit
   //   specialization (14.7.3)
+  // FIXME: We should be checking this for dependent specializations.

erichkeane wrote:

This seems like an important "FIXME' here, right?  Particularly since the 
dependence check is removed below? (10514)

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Krystian Stasiowski via cfe-commits 

sdkrystian wrote:

@erichkeane 
> I guess my first question is 'why'? 

Per the comment you mentioned, it is redundant. Switching to 
`DependentFunctionTemplateSpecializationInfo` also results in constructs such 
as:

```
template
struct A {
  template<>
  void f();
};
```

being diagnosed prior to instantiation. Class scope explicit specializations 
currently exist in their own broken little corner of the frontend, so unifying 
their representation in the AST would be the first step towards properly fixing 
them. 

> My second is 'if this is unnecessary, why was it here in the first place?`. 

`ClassScopeFunctionSpecializationDecl` was originally added in [this 
commit](https://github.com/llvm/llvm-project/commit/00c7e6ceb1826baab50c02f8547e97bfcaf9641c)
 to "parse MSVC standard C++ headers, MFC and ATL code." Back then, class scope 
explicit specializations were a Microsoft extension, but since 
[CWG727](https://wg21.link/cwg727) they are valid ISO C++. As for why this was 
not originally implemented using `DependentFunctionTemplateSpecializationInfo`, 
I cannot say.

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread via cfe-commits 

cor3ntin wrote:

@erichkeane Opinions?

https://github.com/llvm/llvm-project/pull/66636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] remove ClassScopeFunctionSpecializationDecl (PR #66636)

2023-09-18 Thread Krystian Stasiowski via cfe-commits 

https://github.com/sdkrystian created 
https://github.com/llvm/llvm-project/pull/66636

This removes the `ClassScopeFunctionSpecializationDecl` `Decl` node, and 
instead uses `DependentFunctionTemplateSpecializationInfo` to handle such 
declarations. `DependentFunctionTemplateSpecializationInfo` is also changed to 
store a `const ASTTemplateArgumentListInfo*` to be more in line with 
`FunctionTemplateSpecializationInfo`. 

This also changes `FunctionDecl::isFunctionTemplateSpecialization` to return 
`true` for dependent specializations, and 
`FunctionDecl::getTemplateSpecializationKind`/`FunctionDecl::getTemplateSpecializationKindForInstantiation`
 to return `TSK_ExplicitSpecialization` for non-friend dependent 
specializations (the same behavior as dependent class scope 
`ClassTemplateSepcializationDecl` & `VarTemplateSepcializationDecl`).

>From d11d546f3190936ba45c57b4825073026d817878 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski 
Date: Fri, 25 Aug 2023 14:07:32 -0400
Subject: [PATCH] [clang] remove ClassScopeFunctionSpecializationDecl

---
 .../clang-tidy/modernize/UseEmplaceCheck.cpp  |   2 +-
 .../clangd/SemanticHighlighting.cpp   |   9 --
 clang/include/clang/AST/ASTNodeTraverser.h|   7 +-
 clang/include/clang/AST/Decl.h|  10 +-
 clang/include/clang/AST/DeclTemplate.h| 150 +++---
 clang/include/clang/AST/RecursiveASTVisitor.h |  19 ++-
 clang/include/clang/Basic/DeclNodes.td|   1 -
 .../clang/Basic/DiagnosticSemaKinds.td|   8 +-
 clang/include/clang/Sema/Sema.h   |   6 +-
 clang/include/clang/Sema/Template.h   |   2 -
 .../include/clang/Serialization/ASTBitCodes.h |   4 -
 .../clang/Serialization/ASTRecordReader.h |   2 +
 clang/lib/AST/ASTImporter.cpp |  26 ++-
 clang/lib/AST/Decl.cpp|  71 ++---
 clang/lib/AST/DeclBase.cpp|   2 -
 clang/lib/AST/DeclTemplate.cpp|  13 --
 clang/lib/AST/ODRHash.cpp |   4 +
 clang/lib/CodeGen/CGDecl.cpp  |   1 -
 clang/lib/Index/IndexSymbol.cpp   |   1 -
 clang/lib/Sema/SemaDecl.cpp   | 112 ++---
 clang/lib/Sema/SemaTemplate.cpp   |  14 +-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  | 121 +++---
 clang/lib/Serialization/ASTCommon.cpp |   1 -
 clang/lib/Serialization/ASTReader.cpp |  19 ++-
 clang/lib/Serialization/ASTReaderDecl.cpp |  59 ++-
 clang/lib/Serialization/ASTWriter.cpp |   1 -
 clang/lib/Serialization/ASTWriterDecl.cpp |  42 ++---
 .../Checkers/SmartPtrModeling.cpp |   2 +-
 clang/test/AST/ast-dump-decl.cpp  |   9 +-
 clang/test/CXX/drs/dr7xx.cpp  |   5 +
 .../test/SemaTemplate/instantiate-method.cpp  |   7 +-
 clang/tools/libclang/CIndex.cpp   |   1 -
 32 files changed, 290 insertions(+), 441 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index 554abcd900e329c..06ae1f4e257528f 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -67,7 +67,7 @@ AST_MATCHER_P(CallExpr, hasLastArgument,
 // function had parameters defined (this is useful to check if there is only 
one
 // variadic argument).
 AST_MATCHER(CXXMemberCallExpr, hasSameNumArgsAsDeclNumParams) {
-  if (Node.getMethodDecl()->isFunctionTemplateSpecialization())
+  if (Node.getMethodDecl()->getPrimaryTemplate())
 return Node.getNumArgs() == Node.getMethodDecl()
 ->getPrimaryTemplate()
 ->getTemplatedDecl()
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 45c01634e2e38d1..7649e37e1f96663 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -715,13 +715,6 @@ class CollectExtraHighlightings
 return true;
   }
 
-  bool VisitClassScopeFunctionSpecializationDecl(
-  ClassScopeFunctionSpecializationDecl *D) {
-if (auto *Args = D->getTemplateArgsAsWritten())
-  H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
-return true;
-  }
-
   bool VisitDeclRefExpr(DeclRefExpr *E) {
 H.addAngleBracketTokens(E->getLAngleLoc(), E->getRAngleLoc());
 return true;
@@ -752,8 +745,6 @@ class CollectExtraHighlightings
 }
 if (auto *Args = D->getTemplateSpecializationArgsAsWritten())
   H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
-if (auto *I = D->getDependentSpecializationInfo())
-  H.addAngleBracketTokens(I->getLAngleLoc(), I->getRAngleLoc());
 return true;
   }
 
diff --git a/clang/include/clang/AST/ASTNodeTraverser.h 
b/clang/include/clang/AST/ASTNodeTraverser.h
index 1151a756ff377b