[PATCH] D157076: [clang][ExtractAPI] Add support for C++ class templates and concepts

Daniel Grumberg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 16 09:18:51 PDT 2023


dang added a comment.

Looks mostly good. Quick Question how do we handle inheritance to a template parameter?



================
Comment at: clang/include/clang/ExtractAPI/API.h:665
+
+struct ClassTemplateSpecRecord : CXXClassRecord {
+  ClassTemplateSpecRecord(StringRef USR, StringRef Name, PresumedLoc Loc,
----------------
Minor nit, I would prefer for Specialization to be fully spelled out.


================
Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:191
 
+class Template {
+  struct TemplateParameter {
----------------
This is really a model type and should live either in it's own file or in API.h


================
Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:313
+  /// Get template details from a template function, class, or variable
+  static Template getTemplate(const TemplateDecl *Decl) {
+    Template Template;
----------------
I feel this should be a constructor for Template seeing that this never fails.


================
Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:435
+            Decl->getDescribedClassTemplate()));
+    // Cast to easily use previous declaration to get bases.
+    CXXClassRecord = API.addClassTemplate(
----------------
Where is the cast?


================
Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:754
+      Fragments.append(TemplateParam->getTypeConstraint()
+                           ->getNamedConcept()
+                           ->getName()
----------------
is this clang-format formatted?


================
Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:784
+        dyn_cast<TemplateTypeParmDecl>(TemplateParameters[i]);
+    if (TypeParameter.compare("type-parameter-" +
+                              std::to_string(Parameter->getDepth()) + "-" +
----------------
Kinda sad we have to do this. I guess there is no easy way to change the AST to support this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157076/new/

https://reviews.llvm.org/D157076



More information about the cfe-commits mailing list