[PATCH] D155199: [NFC][XCOFF] Use common function to calculate file offset

[Scott Linder via Phabricator via llvm-commits]

scott.linder added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:125
 
+  void advanceFileOffset(const uint64_t &MaxRawDataSize, uint64_t &RawPointer) {
+    FileOffsetToData = RawPointer;
----------------
Generally best to pass primitive scalars like this by value when they are read-only


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:125
 
+  void advanceFileOffset(const uint64_t &MaxRawDataSize, uint64_t &RawPointer) {
+    FileOffsetToData = RawPointer;
----------------
scott.linder wrote:
> Generally best to pass primitive scalars like this by value when they are read-only
As there are already virtual methods I don't think the cost of making this virtual is too high, and it is what I would expect. I'd be surprised if a call to `SectionEntry::advanceFileOffset` was just wrong if the object were a `DwarfSectionEntry`.

Alternatively this method could make a call to a virtual method which returns the "RawSize" of the section, which appears to be different for `DwarfSectionEntry` (i.e. for every other section it is `Size`, for `DwarfSectionEntry` it is `MemorySize`)


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:191
 
 struct DwarfSectionEntry : public SectionEntry {
   // For DWARF section entry.
----------------
I'm not sure why `DwarfSectionEntry` doesn't have a `reset` override which also clears `MemorySize`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155199