[PATCH] D26795: [ELF] Better error reporting for linker scripts

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 21 06:13:19 PST 2016


ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

It's much better than before, but I still think that we could do more by using parallel arrays. We want to maintain four tuples (filename, line content, line number, column number) for each token, and all of them are small (they are intetgers and StringRefs). So I think it's not going to be too much.

That being said, we can do that later. It is indeed a very good improvement. Thanks!

LGTM



================
Comment at: ELF/ScriptParser.cpp:159
+  assert(Pos);
+  return Tokens[Pos - 1];
+}
----------------
I'd rather just use Tokens[Pos - 1] instead of current() everywhere because this function is used from those who already know internals of this class.


================
Comment at: ELF/ScriptParser.h:32
   StringRef peek();
+  StringRef current();
   void skip();
----------------
Make this private.


================
Comment at: ELF/ScriptParser.h:36
   void expect(StringRef Expect);
+  MemoryBufferRef currentBuffer();
 
----------------
Ditto


https://reviews.llvm.org/D26795





More information about the llvm-commits mailing list