[PATCH] D126944: [Clang] Fix memory leak due to TemplateArgumentListInfo used in AST node.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/DeclTemplate.h:2776
   void setTemplateArgsInfo(const TemplateArgumentListInfo &ArgsInfo);
+  void setTemplateArgsInfo(const ASTTemplateArgumentListInfo *ArgsInfo);
 
----------------
Can we make it a prerequisite that the argument has to be nonnull and thus we can pass in a const reference instead?


================
Comment at: clang/include/clang/AST/DeclTemplate.h:2778-2779
 
-  const TemplateArgumentListInfo &getTemplateArgsInfo() const {
+  const ASTTemplateArgumentListInfo *getTemplateArgsInfo() const {
     return TemplateArgsInfo;
   }
----------------
Do we want to assert that `TemplateArgsInfo` is nonnull and return a const reference instead?

Basically, I'm wondering if we can remove a bunch of the pointers and go with references as much as possible; it seems to me that a null pointer is a sign of an error, same as with `TemplateArgs`.


================
Comment at: clang/lib/AST/ASTImporter.cpp:6021
     TemplateArgumentListInfo ToTAInfo;
-    if (Error Err = ImportTemplateArgumentListInfo(
-        D->getTemplateArgsInfo(), ToTAInfo))
-      return std::move(Err);
+    if (auto *Args = D->getTemplateArgsInfo()) {
+      if (Error Err = ImportTemplateArgumentListInfo(
----------------
You should spell out the type name here instead of using `auto` (per coding standard).


================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:5560
+    TemplateArgumentListInfo TemplateArgInfo;
+    if (auto *ArgInfo = VarSpec->getTemplateArgsInfo()) {
+      TemplateArgInfo.setLAngleLoc(ArgInfo->getLAngleLoc());
----------------
You should spell out the type name here as well.


================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:5563-5565
+      for (const TemplateArgumentLoc& arg : ArgInfo->arguments()) {
+        TemplateArgInfo.addArgument(arg);
+      }
----------------
More coding standard changes -- btw, it looks like you need to run the patch through clang-format (https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126944