[PATCH] D19302: ELF: Template LinkerScript class.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Driver.cpp:420
@@ -417,2 +419,3 @@
   SymbolTable<ELFT> Symtab;
+
   std::unique_ptr<TargetInfo> TI(createTarget());
----------------
grimar wrote:
> Is it intentional empty line ?
Yes, as I added two more lines below, I wanted make room between the local variable and local-but-then-global variables.

================
Comment at: ELF/LinkerScript.h:99
@@ -93,1 +98,3 @@
+template <class ELFT> struct Script { static LinkerScript<ELFT> *X; };
+template <class ELFT> LinkerScript<ELFT> *Script<ELFT>::X;
 
----------------
grimar wrote:
> Do you need that struct ?
> LinkerScript itself can be holder for it:
> 
> ```
> // This is a runner of the linker script.
> template <class ELFT> class LinkerScript {
> public:
>    LinkerScript() { X = this; }
>  ...
> 
>    static LinkerScript<ELFT> *X;
> 
> private:
>   ...
> };
> ```
> 
> And we can declare global method to access script:
> ```
> template <class ELFT> LinkerScript<ELFT> *getScript() {
>   return LinkerScript<ELFT>::X;
> }
> ```
> 
> For me using getScript<ELFT>() looks a bit better that 
> accesses through Script<ELFT>::X.
That's an interesting idea, but on the second thought, I decided to go with the shorter name. We'll remove the struct after we migrate to C++14.


http://reviews.llvm.org/D19302