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

[Rui Ueyama via llvm-commits]

ruiu 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) {
----------------
Add "." at the end of the sentence.

================
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.
----------------
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();

================
Comment at: test/ELF/linkerscript-diagnostic.s:68
@@ +67,2 @@
+## Check that last line is "     ^"
+# ERR7DUMP: 00000040 20 20 20 20 20 5e
----------------
I'd use grep instead of hexdump.

  RUN: grep '^     ^' %t.log


http://reviews.llvm.org/D18699