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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits