[Lldb-commits] [PATCH] D140262: [lldb][TypeSystemClang][NFC] Introduce TemplateParameterInfos::ArgumentMetadata

Mon Dec 19 04:45:36 PST 2022

labath added inline comments.

================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:334
+    struct ArgumentMetadata {
+      char const *const name = nullptr;
+    };
----------------
Michael137 wrote:
> Michael137 wrote:
> > labath wrote:
> > > 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.
> > 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
> Whoops, wrong link: https://reviews.llvm.org/D140145
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140262