[PATCH] D21995: [ELF] Implement minimal PHDRS parser and section-to-segment assignment

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:286
@@ +285,3 @@
+  ssize_t RelroNum = -1;
+  Phdr *Load = nullptr;
+  uintX_t Flags = PF_R;
----------------
evgeny777 wrote:
> ssize_t is not (definitly) int.
> It is normally used to hold either size or error.
```
#if defined(_WIN64)
typedef signed __int64 ssize_t;
#else
typedef signed int ssize_t;
#endif /* _WIN64 */

```
It is defined in that way for me.
I think class ErrorOr is used for holding errors in llvm.

We have just a few usings for it in llvm and none in lld at all for that type. I think you want to change it to something different and more consistent with lld code.


================
Comment at: ELF/Writer.h:36
@@ +35,3 @@
+// Each contains type, access flags and range of output sections that will be
+// placed in it.
+template<class ELFT>
----------------
evgeny777 wrote:
> It's been clang-format'ted
I am no doubt it was. But I think it can be 2 lines instead of 3.

================
Comment at: test/ELF/linkerscript-phdrs.s:17
@@ +16,3 @@
+# CHECK-NEXT:      PF_R (0x4)
+# CHECK-NEXT:      PF_W (0x2)
+# CHECK-NEXT:      PF_X (0x1)
----------------
evgeny777 wrote:
> Hmm. Have you tried this:
> ```
> grep PF_R test/ELF/*.s
> ```
> 
Nope. I am windows user I even do not have grep :)
But general rule is not to check something that is not important for the testcase.
We are trying to follow it anyways, even if missed something in previous commits.


http://reviews.llvm.org/D21995