Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][ExtractAPI] improve template argument name deduction #77716

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

evelez7
Copy link
Contributor

@evelez7 evelez7 commented Jan 11, 2024

The names of template arguments in partial specializations or parameters used as types might be mangled according to index and depth. Instead of looping through parameter lists to find matches like we do now, they can be deduced via their QualTypes or as written from the AST.

@llvmbot llvmbot added the clang Clang issues not falling into any other category label Jan 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 11, 2024

@llvm/pr-subscribers-clang

Author: Erick Velez (evelez7)

Changes

The names of template arguments in partial specializations or parameters used as types might be mangled according to index and depth. Instead of looping through parameter lists to find matches like we do now, they can be deduced via their QualTypes or as written from the AST.


Full diff: https://github.com/llvm/llvm-project/pull/77716.diff

4 Files Affected:

  • (modified) clang/include/clang/ExtractAPI/DeclarationFragments.h (+7-15)
  • (modified) clang/lib/ExtractAPI/DeclarationFragments.cpp (+16-42)
  • (modified) clang/test/ExtractAPI/class_template_partial_spec.cpp (+1-2)
  • (modified) clang/test/ExtractAPI/global_var_template_partial_spec.cpp (+1-2)
diff --git a/clang/include/clang/ExtractAPI/DeclarationFragments.h b/clang/include/clang/ExtractAPI/DeclarationFragments.h
index d719196b9a43ec..5af4a01fa6bb9c 100644
--- a/clang/include/clang/ExtractAPI/DeclarationFragments.h
+++ b/clang/include/clang/ExtractAPI/DeclarationFragments.h
@@ -27,8 +27,6 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Lex/MacroInfo.h"
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringRef.h"
 #include <vector>
 
 namespace clang {
@@ -67,6 +65,9 @@ class DeclarationFragments {
     /// parameters.
     GenericParameter,
 
+    /// Argument for partial template specialization.
+    GenericArgument,
+
     /// External parameters in Objective-C methods.
     /// For example, \c forKey in
     /// \code{.m}
@@ -314,13 +315,9 @@ class DeclarationFragmentsBuilder {
   static DeclarationFragments
       getFragmentsForTemplateParameters(ArrayRef<NamedDecl *>);
 
-  static std::string
-  getNameForTemplateArgument(const ArrayRef<NamedDecl *>, std::string);
-
-  static DeclarationFragments
-  getFragmentsForTemplateArguments(const ArrayRef<TemplateArgument>,
-                                   ASTContext &,
-                                   const std::optional<ArrayRef<NamedDecl *>>);
+  static DeclarationFragments getFragmentsForTemplateArguments(
+      const ArrayRef<TemplateArgument>, ASTContext &,
+      const std::optional<ArrayRef<TemplateArgumentLoc>>);
 
   static DeclarationFragments getFragmentsForConcept(const ConceptDecl *);
 
@@ -430,12 +427,7 @@ DeclarationFragmentsBuilder::getFunctionSignature(const FunctionT *Function) {
       dyn_cast<FunctionDecl>(Function)->getDescribedFunctionTemplate() &&
       ReturnType.begin()->Spelling.substr(0, 14).compare("type-parameter") ==
           0) {
-    std::string ProperArgName =
-        getNameForTemplateArgument(dyn_cast<FunctionDecl>(Function)
-                                       ->getDescribedFunctionTemplate()
-                                       ->getTemplateParameters()
-                                       ->asArray(),
-                                   ReturnType.begin()->Spelling);
+    std::string ProperArgName = Function->getReturnType().getAsString();
     ReturnType.begin()->Spelling.swap(ProperArgName);
   }
   ReturnType.append(std::move(After));
diff --git a/clang/lib/ExtractAPI/DeclarationFragments.cpp b/clang/lib/ExtractAPI/DeclarationFragments.cpp
index eb6eea0aaf5465..2c82aeab6083ba 100644
--- a/clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ b/clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -14,14 +14,11 @@
 #include "clang/ExtractAPI/DeclarationFragments.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
-#include "clang/AST/QualTypeNames.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
-#include "clang/Basic/OperatorKinds.h"
 #include "clang/ExtractAPI/TypedefUnderlyingTypeResolver.h"
 #include "clang/Index/USRGeneration.h"
 #include "llvm/ADT/StringSwitch.h"
-#include <typeinfo>
 
 using namespace clang::extractapi;
 using namespace llvm;
@@ -102,6 +99,8 @@ StringRef DeclarationFragments::getFragmentKindString(
     return "internalParam";
   case DeclarationFragments::FragmentKind::Text:
     return "text";
+  case DeclarationFragments::FragmentKind::GenericArgument:
+    return "genericArgument";
   }
 
   llvm_unreachable("Unhandled FragmentKind");
@@ -122,6 +121,8 @@ DeclarationFragments::parseFragmentKindFromString(StringRef S) {
       .Case("internalParam", DeclarationFragments::FragmentKind::InternalParam)
       .Case("externalParam", DeclarationFragments::FragmentKind::ExternalParam)
       .Case("text", DeclarationFragments::FragmentKind::Text)
+      .Case("genericArgument",
+            DeclarationFragments::FragmentKind::GenericArgument)
       .Default(DeclarationFragments::FragmentKind::None);
 }
 
@@ -535,9 +536,7 @@ DeclarationFragmentsBuilder::getFragmentsForVarTemplate(const VarDecl *Var) {
       getFragmentsForType(T, Var->getASTContext(), After);
   if (ArgumentFragment.begin()->Spelling.substr(0, 14).compare(
           "type-parameter") == 0) {
-    std::string ProperArgName = getNameForTemplateArgument(
-        Var->getDescribedVarTemplate()->getTemplateParameters()->asArray(),
-        ArgumentFragment.begin()->Spelling);
+    std::string ProperArgName = T.getAsString();
     ArgumentFragment.begin()->Spelling.swap(ProperArgName);
   }
   Fragments.append(std::move(ArgumentFragment))
@@ -570,12 +569,7 @@ DeclarationFragmentsBuilder::getFragmentsForParam(const ParmVarDecl *Param) {
 
   if (TypeFragments.begin()->Spelling.substr(0, 14).compare("type-parameter") ==
       0) {
-    std::string ProperArgName = getNameForTemplateArgument(
-        dyn_cast<FunctionDecl>(Param->getDeclContext())
-            ->getDescribedFunctionTemplate()
-            ->getTemplateParameters()
-            ->asArray(),
-        TypeFragments.begin()->Spelling);
+    std::string ProperArgName = Param->getOriginalType().getAsString();
     TypeFragments.begin()->Spelling.swap(ProperArgName);
   }
 
@@ -668,11 +662,7 @@ DeclarationFragmentsBuilder::getFragmentsForFunction(const FunctionDecl *Func) {
       getFragmentsForType(Func->getReturnType(), Func->getASTContext(), After);
   if (ReturnValueFragment.begin()->Spelling.substr(0, 14).compare(
           "type-parameter") == 0) {
-    std::string ProperArgName =
-        getNameForTemplateArgument(Func->getDescribedFunctionTemplate()
-                                       ->getTemplateParameters()
-                                       ->asArray(),
-                                   ReturnValueFragment.begin()->Spelling);
+    std::string ProperArgName = Func->getReturnType().getAsString();
     ReturnValueFragment.begin()->Spelling.swap(ProperArgName);
   }
 
@@ -958,25 +948,6 @@ DeclarationFragmentsBuilder::getFragmentsForTemplateParameters(
   return Fragments;
 }
 
-// Find the name of a template argument from the template's parameters.
-std::string DeclarationFragmentsBuilder::getNameForTemplateArgument(
-    const ArrayRef<NamedDecl *> TemplateParameters, std::string TypeParameter) {
-  // The arg is a generic parameter from a partial spec, e.g.
-  // T in template<typename T> Foo<T, int>.
-  //
-  // Those names appear as "type-parameter-<index>-<depth>", so we must find its
-  // name from the template's parameter list.
-  for (unsigned i = 0; i < TemplateParameters.size(); ++i) {
-    const auto *Parameter =
-        dyn_cast<TemplateTypeParmDecl>(TemplateParameters[i]);
-    if (TypeParameter.compare("type-parameter-" +
-                              std::to_string(Parameter->getDepth()) + "-" +
-                              std::to_string(Parameter->getIndex())) == 0)
-      return std::string(TemplateParameters[i]->getName());
-  }
-  llvm_unreachable("Could not find the name of a template argument.");
-}
-
 // Get fragments for template arguments, e.g. int in template<typename T>
 // Foo<int>;
 //
@@ -986,7 +957,7 @@ std::string DeclarationFragmentsBuilder::getNameForTemplateArgument(
 DeclarationFragments
 DeclarationFragmentsBuilder::getFragmentsForTemplateArguments(
     const ArrayRef<TemplateArgument> TemplateArguments, ASTContext &Context,
-    const std::optional<ArrayRef<NamedDecl *>> TemplateParameters) {
+    const std::optional<ArrayRef<TemplateArgumentLoc>> TemplateArgumentLocs) {
   DeclarationFragments Fragments;
   for (unsigned i = 0, end = TemplateArguments.size(); i != end; ++i) {
     if (i)
@@ -1000,9 +971,12 @@ DeclarationFragmentsBuilder::getFragmentsForTemplateArguments(
 
     if (ArgumentFragment.begin()->Spelling.substr(0, 14).compare(
             "type-parameter") == 0) {
-      std::string ProperArgName = getNameForTemplateArgument(
-          TemplateParameters.value(), ArgumentFragment.begin()->Spelling);
-      ArgumentFragment.begin()->Spelling.swap(ProperArgName);
+      std::string ProperArgName = TemplateArgumentLocs.value()[i]
+                                      .getTypeSourceInfo()
+                                      ->getType()
+                                      .getAsString();
+      ArgumentFragment = DeclarationFragments().append(
+          ProperArgName, DeclarationFragments::FragmentKind::GenericArgument);
     }
     Fragments.append(std::move(ArgumentFragment));
 
@@ -1086,7 +1060,7 @@ DeclarationFragmentsBuilder::getFragmentsForClassTemplatePartialSpecialization(
       .append("<", DeclarationFragments::FragmentKind::Text)
       .append(getFragmentsForTemplateArguments(
           Decl->getTemplateArgs().asArray(), Decl->getASTContext(),
-          Decl->getTemplateParameters()->asArray()))
+          Decl->getTemplateArgsAsWritten()->arguments()))
       .append(">", DeclarationFragments::FragmentKind::Text)
       .append(";", DeclarationFragments::FragmentKind::Text);
 }
@@ -1127,7 +1101,7 @@ DeclarationFragmentsBuilder::getFragmentsForVarTemplatePartialSpecialization(
       .append("<", DeclarationFragments::FragmentKind::Text)
       .append(getFragmentsForTemplateArguments(
           Decl->getTemplateArgs().asArray(), Decl->getASTContext(),
-          Decl->getTemplateParameters()->asArray()))
+          Decl->getTemplateArgsAsWritten()->arguments()))
       .append(">", DeclarationFragments::FragmentKind::Text)
       .append(";", DeclarationFragments::FragmentKind::Text);
 }
diff --git a/clang/test/ExtractAPI/class_template_partial_spec.cpp b/clang/test/ExtractAPI/class_template_partial_spec.cpp
index eba069319ce450..fc21d61297e4b9 100644
--- a/clang/test/ExtractAPI/class_template_partial_spec.cpp
+++ b/clang/test/ExtractAPI/class_template_partial_spec.cpp
@@ -196,8 +196,7 @@ template<typename Z> class Foo<Z, int> {};
           "spelling": "<"
         },
         {
-          "kind": "typeIdentifier",
-          "preciseIdentifier": "c:t0.0",
+          "kind": "genericArgument",
           "spelling": "Z"
         },
         {
diff --git a/clang/test/ExtractAPI/global_var_template_partial_spec.cpp b/clang/test/ExtractAPI/global_var_template_partial_spec.cpp
index e98076cdb1d016..cca571ff1999fe 100644
--- a/clang/test/ExtractAPI/global_var_template_partial_spec.cpp
+++ b/clang/test/ExtractAPI/global_var_template_partial_spec.cpp
@@ -206,8 +206,7 @@ template<typename Z> int Foo<int, Z> = 0;
           "spelling": ", "
         },
         {
-          "kind": "typeIdentifier",
-          "preciseIdentifier": "c:t0.0",
+          "kind": "genericArgument",
           "spelling": "Z"
         },
         {

@evelez7
Copy link
Contributor Author

evelez7 commented Feb 8, 2024

ping

@@ -1127,7 +1096,7 @@ DeclarationFragmentsBuilder::getFragmentsForVarTemplatePartialSpecialization(
.append("<", DeclarationFragments::FragmentKind::Text)
.append(getFragmentsForTemplateArguments(
Decl->getTemplateArgs().asArray(), Decl->getASTContext(),
Decl->getTemplateParameters()->asArray()))
Decl->getTemplateArgsAsWritten()->arguments()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the right thing to me!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this patch getFragmentsForTemplateArguments, which is being called here, is changed to accept ArrayRef<TemplateArgumentLoc> (which is what the changed line returns) instead of ArrayRef<NamedDecl *> because of the nasty loop that was deleted in DeclarationFragments.cpp:962. A TemplateArgumentLoc gives us the name of the specialization's type argument directly, instead of comparing with the declaration's template parameters.

The names of template arguments in partial specializations or parameters used as
types might be mangled according to index and depth. Instead of looping
through parameter lists to find matches like we do now, they can be
deduced via their QualTypes or as written from the AST.
@evelez7 evelez7 force-pushed the extractapi-better-template-args branch from c6373dc to f0f25f2 Compare March 27, 2024 02:52
@daniel-grumberg daniel-grumberg self-requested a review April 2, 2024 14:09
@evelez7 evelez7 merged commit 2b6c038 into llvm:main Apr 2, 2024
4 checks passed
daniel-grumberg pushed a commit to daniel-grumberg/llvm-project that referenced this pull request Apr 3, 2024
)

The names of template arguments in partial specializations or parameters
used as types might be mangled according to index and depth. Instead of
looping through parameter lists to find matches like we do now, they can
be deduced via their QualTypes or as written from the AST.
@evelez7 evelez7 deleted the extractapi-better-template-args branch June 3, 2024 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants