[PATCH] D18699: [ELF] - Teach linkerscript error handler to show full script line + column marker on error.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:176
@@ -174,1 +175,3 @@
 
+// Returns the line that the character S[Pos] is in
+static StringRef getLine(StringRef S, size_t Pos) {
----------------
ruiu wrote:
> Add "." at the end of the sentence.
Done.

================
Comment at: ELF/LinkerScript.cpp:182
@@ +181,3 @@
+  if (End == StringRef::npos)
+    End = S.size();
+  // rtrim() needed here because string can have '\r\n' at end.
----------------
ruiu wrote:
> Well, maybe the tone of this comment made me think of that. It is not "neeeded" on Unix, but it's good just in case. I'd update the comment.
> 
>   // rtrim for DOS-style newlines.
>   return S.substr(Begin, End - Begin).rtrim();
I will be more careful with wording next time. Fixed.

================
Comment at: test/ELF/linkerscript-diagnostic.s:68
@@ +67,2 @@
+## Check that last line is "     ^"
+# ERR7DUMP: 00000040 20 20 20 20 20 5e
----------------
ruiu wrote:
> I'd use grep instead of hexdump.
> 
>   RUN: grep '^     ^' %t.log
Works like a charm ! Thanks for hint.


http://reviews.llvm.org/D18699