[PATCH] Separate file parsing from File's constructors.
Rui Ueyama
ruiu at google.com
Thu Dec 11 19:53:30 PST 2014
================
Comment at: include/lld/Core/File.h:156-157
@@ -155,1 +155,4 @@
+ virtual std::error_code parse() { return std::error_code(); }
+ void setParsed(bool val) { _parsed = val; }
+
----------------
shankarke wrote:
> Can you add doxygen comments here.
Done
================
Comment at: include/lld/Core/File.h:218
@@ -213,2 +217,3 @@
static atom_collection_empty<AbsoluteAtom> _noAbsoluteAtoms;
+ bool _parsed;
mutable llvm::BumpPtrAllocator _allocator;
----------------
shankarke wrote:
> There is only one state being tracked, what about parsed but there was an error ? If there is a call that calls the parse function again, it will return success.
That is what Michael pointed out. Made a change to save the last error code instead of a boolean value
================
Comment at: lib/ReaderWriter/ELF/ELFFile.h:422
@@ -422,4 +421,3 @@
new ELFFile<ELFT>(std::move(mb), atomizeStrings));
- if (std::error_code ec = file->parse())
- return ec;
+ file->_mb = std::move(mb);
return std::move(file);
----------------
shankarke wrote:
> Why std::move again ?
Removed.
================
Comment at: lib/ReaderWriter/MachO/File.h:212
@@ -192,1 +211,3 @@
+ MemoryBuffer *_mb;
+ MachOLinkingContext *_ctx;
----------------
shankarke wrote:
> should this be a unique_ptr ?
I was tempted to make this a std::unique_ptr but decided to not do that. This is the mechanical translation of the original code. If this has to be fixed, it should be done in a followup patch.
http://reviews.llvm.org/D6633
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list