[PATCH] D70930: [llvm-objcopy][WebAssembly] Initial support for wasm in llvm-objcopy

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 17 17:09:25 PST 2019


alexshap added a comment.

okay, thank you again for the patience, overall this looks cleaner to me



================
Comment at: llvm/tools/llvm-objcopy/wasm/Reader.cpp:26
+  }
+  Obj->addSections(Sections);
+  return std::move(Obj);
----------------
sorry, I didn't notice this before - but this is one reason why I wasn't a big fan of the proposed interface,
adding a section (or sections) always involves a copy.
For example, if vector<Section> Sections  was just a public field of Object 
(or Object was a struct  (it still can have a method removeSections if necessary)) 
you would simply do the following:
     Obj.Sections.reserve(WasmObj.sections().size());
     for (SectionRef Sec : WasobObj.sections()) {
          Sections.push_back(... );
     }
Frankly speaking it still feels like addSections(...) doesn't buy us much. (and the same is true for getSections)


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