[PATCH] D19302: ELF: Template LinkerScript class.

[George Rimar via llvm-commits]

grimar added a comment.

Thats looks as nice cleanup change. Have some suggestions, 
thought not sure how much them are reasonable.


================
Comment at: ELF/Driver.cpp:420
@@ -417,2 +419,3 @@
   SymbolTable<ELFT> Symtab;
+
   std::unique_ptr<TargetInfo> TI(createTarget());
----------------
Is it intentional empty line ?

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


http://reviews.llvm.org/D19302