[PATCH] D97657: [lld][WebAssembly] Initial support merging string data

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: lld/wasm/InputChunks.h:216
+public:
+  SyntheticMergedDataSegment(StringRef name, uint32_t flags_,
+                             uint32_t alignment_)
----------------
dblaikie wrote:
> sbc100 wrote:
> > dschuff wrote:
> > > Do we care about these lint checks? (I guess lld already has different style than LLVM?)
> > We do care yes, but I'm having hard time seeing what is wrong here since we use lowerCamal case for locals and params in lld.   i guess it must be the trailing `_` .. in which case maybe its false positive because it thinks we are using snake_case which would indeed be wrong.   I think we can ignore this one.
> What's with the underscores though?
> 
> I don't think we usually do that in the LLVM codebase - I think we generally name the parameter the same as the variable & ideally use the ctor's initializer list to initialize members. (or use `this->x = x` in a pinch)
> 
> Hmm, I guess these members are members of a base class? Perhaps the base class should have a ctor that takes them?
In this case I'm initialize members of that superclass so I can't use the initializer list (I think).. so I need to disambiguate..   In the past I've used the  underscore suffix disambiguate, but I'm happy to use the `this->x = x` method instead.  I didn't know that was preferred here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97657