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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lld/wasm/InputChunks.h:216
+public:
+  SyntheticMergedDataSegment(StringRef name, uint32_t flags_,
+                             uint32_t alignment_)
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97657