[PATCH] D70930: [llvm-objcopy][WebAssembly] Initial support for wasm in llvm-objcopy
Derek Schuff via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 23 09:05:56 PST 2020
dschuff added inline comments.
================
Comment at: llvm/tools/llvm-objcopy/wasm/Writer.cpp:24-25
+// The header consists of
+// * A one-byte section ID (aka the section type)
+// * The size of the section contents, encoded as ULEB128
+// * If the section is a custom section (type 0) it also has a name, which is
----------------
jhenderson wrote:
> Missing trailing full stops on these two lines.
done (but these are intentionally not complete sentences).
================
Comment at: llvm/tools/llvm-objcopy/wasm/Writer.cpp:31
+// Return the header and store the total size in SectionSize.
+// static
+Writer::SectionHeader Writer::generateSectionHeader(const Section &S,
----------------
jhenderson wrote:
> `// static`?
This is just the common annotation indicating that `generateSectionHeader` is a static method. I added a blank line to make it more clear that it goes with the method definition rather than the block comment above.
================
Comment at: llvm/tools/llvm-objcopy/wasm/Writer.h:32-33
+
+ static SectionHeader generateSectionHeader(const Section &S,
+ size_t &SectionSize);
+ size_t finalize();
----------------
jhenderson wrote:
> Not a big deal, but any particular reason you've called this `generateSectionHeader` instead of the shorter `createSectionHender`?
No particular reason; habit from working on code generators I suppose. I changed it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70930/new/
https://reviews.llvm.org/D70930
More information about the llvm-commits
mailing list