[PATCH] D23872: [ELF] - Linkerscript: simplify access to templated methods from parser.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 31 03:38:50 PDT 2016


grimar added a comment.

In https://reviews.llvm.org/D23872#529252, @ruiu wrote:

> As I wrote in the comment to the other patch, I think I still prefer the current way than the way you are suggesting in this patch. The current approach seems silly but simple.


I see. But there are 4 methods already and probably we`ll need more. Each one use of non-template stuff is an excessive call redirection and always a new method with bunch of code.
Virtual calls for linkerscript tasks costs nothing at the same time.

I updated this patch just in case to show how much code is removed from head revision.


================
Comment at: ELF/LinkerScript.h:140
@@ -132,1 +139,3 @@
+
+  ScriptMethods* Methods;
 };
----------------
rafael wrote:
> This is the same as Script<ELFT>::X, just with a non templated type, no?
> 
> Can you rename ScriptMethods to LinkerScriptBase and replace  Script<ELFT>::X with a "LinkerScriptBase *Script"?
> 
Not sure I understand the idea correctly. If I replace usings of Script<ELFT>::X with a "LinkerScriptBase *Script",
I`ll need to add more virtual methods to the LinkerScriptBase. And that is not possible for all of them, because few
are templated:

```
  std::vector<PhdrEntry<ELFT>> createPhdrs();
  bool shouldKeep(InputSectionBase<ELFT> *S);
```

I tried to do:
```
extern LinkerScriptBase* ScriptBase;
template <class ELFT> LinkerScript<ELFT> *getScript() {
  return static_cast<LinkerScript<ELFT> *>(ScriptBase);
}
```
Instead of:
```
template <class ELFT> struct Script { static LinkerScript<ELFT> *X; };
template <class ELFT> LinkerScript<ELFT> *Script<ELFT>::X;
```
But use of getScript<ELFT() everywhere in the code introduces lots of minor changes,
not sure it looks nicer either.

I ended up with adding single global variable.


https://reviews.llvm.org/D23872





More information about the llvm-commits mailing list