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

[Andrew via Phabricator via cfe-commits]

browneee added inline comments.


================
Comment at: clang/include/clang/AST/DeclTemplate.h:2776
   void setTemplateArgsInfo(const TemplateArgumentListInfo &ArgsInfo);
+  void setTemplateArgsInfo(const ASTTemplateArgumentListInfo *ArgsInfo);
 
----------------
aaron.ballman wrote:
> Can we make it a prerequisite that the argument has to be nonnull and thus we can pass in a const reference instead?
I tried this initially. Tests broke, so I went with this more conservative change. Someone may be able to reduce use of nulls in follow up changes.


================
Comment at: clang/include/clang/AST/DeclTemplate.h:2778-2779
 
-  const TemplateArgumentListInfo &getTemplateArgsInfo() const {
+  const ASTTemplateArgumentListInfo *getTemplateArgsInfo() const {
     return TemplateArgsInfo;
   }
----------------
aaron.ballman wrote:
> 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`.
This may be why https://reviews.llvm.org/D126937 is failing tests.

My goals is to fix the memory leak, so the sanitizer buildbots are clean. I would agree there is probably more cleanup to do for this member variable after https://reviews.llvm.org/D126944 - but fixing the memory leak is the urgent thing for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126944