[PATCH] D14382: [ELF2] - Basic implementation of -r/--relocatable

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/OutputSections.cpp:763
@@ +762,3 @@
+  else if (RelocOutSec != Sec->RelocatingSection->OutSec)
+    error("Sections to which the relocations applies doesn`t match");
+
----------------
ruiu wrote:
> grimar wrote:
> > rafael wrote:
> > > Can you get here? If so, add a test, if not, use an assert.
> > I just assumed that users who will try linker probably will use the release version. So if them meet that error situation, there is no way they see the assert, it will not be debug build for them.
> > error() is universal way to prompt here. I would use it or fatal() here.
> So, the point is that
> 
>  - if this is not reachable except by programmer's error (by a bug), assert is better
>  - if this is reachable by feeding incompatible files, giving incorrect combination of flags, etc, error is appropriate
Yep, that was clear for me. I just not always sure which situations are reachable by feeding incompatible files for example. 'Corrupted' files can contain absolutely anything to trigger different unexpected situations.
Replaced with assert as I dont know way how to reach here.

================
Comment at: ELF/Writer.cpp:844
@@ -836,2 +843,3 @@
   case InputSectionBase<ELFT>::Regular:
-    Sec = new OutputSection<ELFT>(Key.Name, Key.Type, Key.Flags);
+    if (C->RelocatingSection)
+      Sec = new StaticRelocSection<ELFT>(Key.Name, Key.Type, Key.Flags,
----------------
ruiu wrote:
> Checking for `RelocatingSection` to see if an input section is a relocation section is an obscure way to distinguish two different kind of sections. Maybe we should define RelInputSection for relocation sections?
Done. May be RelInput would be better ?
Then we would have:

```
enum Kind { Regular, RelInput, EHFrame, Merge, MipsReginfo };
```
I mean all of that are sections kind, IMO no need to specify "section" in enum name again.


http://reviews.llvm.org/D14382