[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