[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks fine. A bit of cleanup might be nice on the TypeSystem.h to provide default implementations that just return 0 for some of these that every type system currently is required to override for no reason (all template related type system calls seem to have default "return 0;" functions in each type system. We can make future changes to these touch fewer files if we provide default implementations in TypeSystem.h.



================
Comment at: include/lldb/Symbol/CompilerType.h:367-376
+  size_t GetNumTemplateArguments(bool expand_pack = false) const;
 
-  lldb::TemplateArgumentKind GetTemplateArgumentKind(size_t idx) const;
-  CompilerType GetTypeTemplateArgument(size_t idx) const;
+  lldb::TemplateArgumentKind
+  GetTemplateArgumentKind(size_t idx, bool expand_pack = false) const;
+  CompilerType GetTypeTemplateArgument(size_t idx,
+                                       bool expand_pack = false) const;
 
----------------
I know there wasn't documentation on these functions, but it might be nice to docuement what "expand_pack" means.


================
Comment at: include/lldb/Symbol/GoASTContext.h:313-316
+  size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
+                                 bool expand_pack) override {
     return 0;
   }
----------------
We should make a default implementation of this function in TypeSystem.h so not every TypeSystem subclass needs to make a do nothing and return 0 function. It will make future changes easier and touch less files.


================
Comment at: include/lldb/Symbol/OCamlASTContext.h:230-233
+  size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
+                                 bool expand_pack) override {
     return 0;
   }
----------------
We should make a default implementation of this function in TypeSystem.h so not every TypeSystem subclass needs to make a do nothing and return 0 function. It will make future changes easier and touch less files.


================
Comment at: source/Symbol/JavaASTContext.cpp:881-885
 size_t
-JavaASTContext::GetNumTemplateArguments(lldb::opaque_compiler_type_t type) {
+JavaASTContext::GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
+                                        bool expand_pack) {
   return 0;
 }
----------------
We should make a default implementation of this function in TypeSystem.h so not every TypeSystem subclass needs to make a do nothing and return 0 function. It will make future changes easier and touch less files.


https://reviews.llvm.org/D51387