[lldb][TypeSystemClang][NFC] Introduce TemplateParameterInfos::ArgumentMetadata

[Michael137]

Summary

This patch creates a new aggregate type that holds
information about a template argument. Currently this
is only its name but in future patches we plan to add
additional information.

[labath]

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

334

This may depend on where you're going with this, but right now it seems like this const here is overkill, as one can express the notion of "const metadata" by making the whole ArgumentMetadata class const.

That obviously won't work if your actual goal is to have some kind of "mutable" metadata, but then I'd like to hear more about what is that going to be used for.

[Michael137]

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

334

Fair point

The use-case is the follow-up PR here: https://reviews.llvm.org/D140262 which is still WIP

Depending on whether that works out we may or may not need this patch here in the first place. So I'm holding off of committing it for now

334

Whoops, wrong link: https://reviews.llvm.org/D140145

[labath]

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

334

Storing the is_default flag here makes sense to me, but I feel the same way about it's constness as I feel about the name field. Since ArgumentMetadata is just a dumb container, I don't think it makes sense enforcing constness at this level. If we want to, we can always make individual instances of this struct const as a whole.

[Michael137]

Not needed anymore due to simpler implementation at https://reviews.llvm.org/D141828

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

CompilerType CreateRecordType(clang::DeclContext *decl_ctx,
                                 lldb::AccessType access_type,
                                 llvm::StringRef name, int kind,
                                 lldb::LanguageType language,
                                 ClangASTMetadata *metadata = nullptr,
                                 bool exports_symbols = false);
 
   class TemplateParameterInfos {
   public:
+    struct ArgumentMetadata {
+      char const *const name = nullptr;
+    };
+
     TemplateParameterInfos() = default;
-    TemplateParameterInfos(llvm::ArrayRef<const char *> names_in,
+    TemplateParameterInfos(llvm::ArrayRef<ArgumentMetadata> metadata,
                            llvm::ArrayRef<clang::TemplateArgument> args_in)
-        : names(names_in), args(args_in) {
-      assert(names.size() == args_in.size());
+        : args_metadata(metadata), args(args_in) {
+      assert(metadata.size() == args_in.size());
     }
 
     TemplateParameterInfos(TemplateParameterInfos const &) = delete;
     TemplateParameterInfos(TemplateParameterInfos &&) = delete;
 
     TemplateParameterInfos &operator=(TemplateParameterInfos const &) = delete;
     TemplateParameterInfos &operator=(TemplateParameterInfos &&) = delete;
 
     ~TemplateParameterInfos() = default;
 
     bool IsValid() const {
       // Having a pack name but no packed args doesn't make sense, so mark
       // these template parameters as invalid.
       if (pack_name && !packed_args)
         return false;
-      return args.size() == names.size() &&
+      return args.size() == args_metadata.size() &&
              (!packed_args || !packed_args->packed_args);
     }
 
     bool IsEmpty() const { return args.empty(); }
     size_t Size() const { return args.size(); }
 
     llvm::ArrayRef<clang::TemplateArgument> GetArgs() const { return args; }
-    llvm::ArrayRef<const char *> GetNames() const { return names; }
+    llvm::ArrayRef<ArgumentMetadata> GetMetadata() const {
+      return args_metadata;
+    }
 
     clang::TemplateArgument const &Front() const {
       assert(!args.empty());
       return args.front();
     }
 
     void InsertArg(char const *name, clang::TemplateArgument arg) {
       args.emplace_back(std::move(arg));
-      names.push_back(name);
+      args_metadata.push_back({name});
     }
 
     // Parameter pack related
 
     bool hasParameterPack() const { return static_cast<bool>(packed_args); }
 
     TemplateParameterInfos const &GetParameterPack() const {
       assert(packed_args != nullptr);
public:
 
     void SetPackName(char const *name) { pack_name = name; }
 
     void SetParameterPack(std::unique_ptr<TemplateParameterInfos> args) {
       packed_args = std::move(args);
     }
 
   private:
-    /// Element 'names[i]' holds the template argument name
+    /// Element 'args_metadata[i]' holds the template argument metadata
     /// of 'args[i]'
-    llvm::SmallVector<const char *, 2> names;
+    llvm::SmallVector<ArgumentMetadata, 2> args_metadata;
     llvm::SmallVector<clang::TemplateArgument, 2> args;
 
     const char * pack_name = nullptr;
     std::unique_ptr<TemplateParameterInfos> packed_args;
   };
 
   clang::FunctionTemplateDecl *CreateFunctionTemplateDecl(
       clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module,

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

static TemplateParameterList *CreateTemplateParameterList(
   const bool parameter_pack = false;
   const bool is_typename = false;
   const unsigned depth = 0;
   const size_t num_template_params = template_param_infos.Size();
   DeclContext *const decl_context =
       ast.getTranslationUnitDecl(); // Is this the right decl context?,
 
   auto const &args = template_param_infos.GetArgs();
-  auto const &names = template_param_infos.GetNames();
+  auto const &metadata = template_param_infos.GetMetadata();
   for (size_t i = 0; i < num_template_params; ++i) {
-    const char *name = names[i];
+    const char *name = metadata[i].name;
 
     IdentifierInfo *identifier_info = nullptr;
     if (name && name[0])
       identifier_info = &ast.Idents.get(name);
     TemplateArgument const &targ = args[i];
     if (IsValueParam(targ)) {
       QualType template_param_type = targ.getIntegralType();
       template_param_decls.push_back(NonTypeTemplateParmDecl::Create(

lldb/unittests/Symbol/TestTypeSystemClang.cpp

clang::TemplateArgument charArg1(m_ast->getASTContext(),
                                    m_ast->getASTContext().SignedCharTy);
 
   clang::TemplateArgument charArg2(m_ast->getASTContext(),
                                    llvm::APSInt(llvm::APInt(8, 123)),
                                    m_ast->getASTContext().SignedCharTy);
 
   // Test that <typename T> with T = int creates a new template.
   ClassTemplateDecl *single_type_arg =
-      ExpectNewTemplate("<typename T>", {{"T"}, {intArg1}});
+      ExpectNewTemplate("<typename T>", {{{"T"}}, {intArg1}});
 
   // Test that changing the parameter name doesn't create a new class template.
-  ExpectReusedTemplate("<typename A> (A = int)", {{"A"}, {intArg1}},
+  ExpectReusedTemplate("<typename A> (A = int)", {{{"A"}}, {intArg1}},
                        single_type_arg);
 
   // Test that changing the used type doesn't create a new class template.
-  ExpectReusedTemplate("<typename A> (A = float)", {{"A"}, {floatArg}},
+  ExpectReusedTemplate("<typename A> (A = float)", {{{"A"}}, {floatArg}},
                        single_type_arg);
 
   // Test that <typename A, signed char I> creates a new template with A = int
   // and I = 47;
   ClassTemplateDecl *type_and_char_value =
       ExpectNewTemplate("<typename A, signed char I> (I = 47)",
-                        {{"A", "I"}, {floatArg, charArg1}});
+                        {{{"A"}, {"I"}}, {floatArg, charArg1}});
 
   // Change the value of the I parameter to 123. The previously created
   // class template should still be reused.
   ExpectReusedTemplate("<typename A, signed char I> (I = 123)",
-                       {{"A", "I"}, {floatArg, charArg2}}, type_and_char_value);
+                       {{{"A"}, {"I"}}, {floatArg, charArg2}},
+                       type_and_char_value);
 
   // Change the type of the I parameter to int so we have <typename A, int I>.
   // The class template from above can't be reused.
   ExpectNewTemplate("<typename A, int I> (I = 123)",
-                    {{"A", "I"}, {floatArg, intArg2}});
+                    {{{"A"}, {"I"}}, {floatArg, intArg2}});
 
   // Test a second type parameter will also cause a new template to be created.
   // We now have <typename A, int I, typename B>.
   ClassTemplateDecl *type_and_char_value_and_type =
       ExpectNewTemplate("<typename A, int I, typename B>",
-                        {{"A", "I", "B"}, {floatArg, intArg2, intArg1}});
+                        {{{"A"}, {"I"}, {"B"}}, {floatArg, intArg2, intArg1}});
 
   // Remove all the names from the parameters which shouldn't influence the
   // way the templates get merged.
   ExpectReusedTemplate("<typename, int, typename>",
-                       {{"", "", ""}, {floatArg, intArg2, intArg1}},
+                       {{{""}, {""}, {""}}, {floatArg, intArg2, intArg1}},
                        type_and_char_value_and_type);
 }
 
 TEST_F(TestCreateClassTemplateDecl, FindExistingTemplatesWithParameterPack) {
   // The same as FindExistingTemplates but for templates with parameter packs.
+
+  using ArgumentMetadata =
+      TypeSystemClang::TemplateParameterInfos::ArgumentMetadata;
+
   TypeSystemClang::TemplateParameterInfos infos;
   clang::TemplateArgument intArg1(m_ast->getASTContext().IntTy);
   clang::TemplateArgument intArg2(m_ast->getASTContext(),
                                   llvm::APSInt(llvm::APInt(32, 1)),
                                   m_ast->getASTContext().IntTy);
   clang::TemplateArgument intArg3(m_ast->getASTContext(),
                                   llvm::APSInt(llvm::APInt(32, 123)),
                                   m_ast->getASTContext().IntTy);
   clang::TemplateArgument longArg1(m_ast->getASTContext().LongTy);
   clang::TemplateArgument longArg2(m_ast->getASTContext(),
                                    llvm::APSInt(llvm::APInt(64, 1)),
                                    m_ast->getASTContext().LongTy);
 
   infos.SetParameterPack(
       std::make_unique<TypeSystemClang::TemplateParameterInfos>(
-          llvm::SmallVector<const char *>{"", ""},
+          llvm::SmallVector<ArgumentMetadata>{{""}, {""}},
           llvm::SmallVector<TemplateArgument>{intArg1, intArg1}));
 
   ClassTemplateDecl *type_pack =
       ExpectNewTemplate("<typename ...> (int, int)", infos);
 
   // Special case: An instantiation for a parameter pack with no values fits
   // to whatever class template we find. There isn't enough information to
   // do an actual comparison here.
   infos.SetParameterPack(
       std::make_unique<TypeSystemClang::TemplateParameterInfos>());
   ExpectReusedTemplate("<...> (no values in pack)", infos, type_pack);
 
   // Change the type content of pack type values.
   infos.SetParameterPack(
       std::make_unique<TypeSystemClang::TemplateParameterInfos>(
-          llvm::SmallVector<const char *>{"", ""},
+          llvm::SmallVector<ArgumentMetadata>{{""}, {""}},
           llvm::SmallVector<TemplateArgument>{intArg1, longArg1}));
   ExpectReusedTemplate("<typename ...> (int, long)", infos, type_pack);
 
   // Change the number of pack values.
   infos.SetParameterPack(
       std::make_unique<TypeSystemClang::TemplateParameterInfos>(
-          llvm::SmallVector<const char *>{""},
+          llvm::SmallVector<ArgumentMetadata>{{""}},
           llvm::SmallVector<TemplateArgument>{intArg1}));
   ExpectReusedTemplate("<typename ...> (int)", infos, type_pack);
 
   // The names of the pack values shouldn't matter.
   infos.SetParameterPack(
       std::make_unique<TypeSystemClang::TemplateParameterInfos>(
-          llvm::SmallVector<const char *>{"A"},
+          llvm::SmallVector<ArgumentMetadata>{{"A"}},
           llvm::SmallVector<TemplateArgument>{intArg1}));
   ExpectReusedTemplate("<typename ...> (int)", infos, type_pack);
 
   // Changing the kind of template argument will create a new template.
   infos.SetParameterPack(
       std::make_unique<TypeSystemClang::TemplateParameterInfos>(
-          llvm::SmallVector<const char *>{"A"},
+          llvm::SmallVector<ArgumentMetadata>{{"A"}},
           llvm::SmallVector<TemplateArgument>{intArg2}));
   ClassTemplateDecl *int_pack = ExpectNewTemplate("<int ...> (int = 1)", infos);
 
   // Changing the value of integral parameters will not create a new template.
   infos.SetParameterPack(
       std::make_unique<TypeSystemClang::TemplateParameterInfos>(
-          llvm::SmallVector<const char *>{"A"},
+          llvm::SmallVector<ArgumentMetadata>{{"A"}},
           llvm::SmallVector<TemplateArgument>{intArg3}));
   ExpectReusedTemplate("<int ...> (int = 123)", infos, int_pack);
 
   // Changing the integral type will create a new template.
   infos.SetParameterPack(
       std::make_unique<TypeSystemClang::TemplateParameterInfos>(
-          llvm::SmallVector<const char *>{"A"},
+          llvm::SmallVector<ArgumentMetadata>{{"A"}},
           llvm::SmallVector<TemplateArgument>{longArg2}));
   ExpectNewTemplate("<long ...> (long = 1)", infos);
 
   // Prependinding a non-pack parameter will create a new template.
   infos.InsertArg("T", intArg1);
   ExpectNewTemplate("<typename T, long...> (T = int, long = 1)", infos);
 }