[PATCH] D16771: [ELF] PHDRS linker script command implemented.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:358
@@ +357,3 @@
+    error("Integer can't be parsed: " + Tok);
+  return Val;
+}
----------------
grimar wrote:
> ruiu wrote:
> > Do not return an undefined value.
> That was done intentionally. My point was that using of the return value in case of error is also a error (at least for that method), error flag should be checked, value can be accessedm but should not be used in any results because of invalid nature. Undefined value accepts to this requirements.
> When you look the value of undefined variable it is usually clear it is the one, but defining it to something can hide the actual error.
Returning an undefined variable is an undefined behavior, no? You cannot assume that an undefined value can be identified as such easily. In some compiler, it may, it totally depends on your environment. Since it is an undefined behavior, I think the compiler can even emit code to terminate the program at end of this function.

================
Comment at: ELF/LinkerScript.cpp:380
@@ +379,3 @@
+      else
+        error("Unknown header attribute " + Tok);
+    }
----------------
grimar wrote:
> ruiu wrote:
> > Handle errors gracefully.
> Sorry, not sure I see something wrong here also. skip() has a check for error flag, so if something wrong here, it will just exit finally from both of while loops. There is no need of excessive error flags here and that is why I think it is gracefully already.
You need to check `Error` and exit from this loop. Currently I think this is an infinite loop if there is an opening '{' but no closing '}'.


http://reviews.llvm.org/D16771