[PATCH] D45375: [ELF] - Introduce synthetic file for linker script symbols.

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I agree, I think that a synthetic file has some benefits over using nullptr. I have worked on a linker that did that in the past and it was a useful property to have if no Symbol had a nullptr File. There were cases where one slipped through and it caused a crash, so some extra asserts may be needed.

Some comments on this particular implementation:

- I suggest not creating separate files for each linkerscript defined symbol, with the location encoded in the filename. To me that isn't what I'd expect if I asked for the filename, I'd expect just the filename of the linkerscript that contained it. If we want to have extra location information such as line numbers for linker scripts, or Section + Offset etc. for symbols defined in objects we can add them on via some other means. Having a single file per linker script has some advantages in comparing symbols based on which InputFile defined them. Ideally we'd want two symbols defined in the same linkerscript to have the same file.
- I'd also create an <internal> synthetic file for the linker defined symbols like _GLOBAL_OFFSET_ table where there isn't a real file. That would permit us to assume that the file wouldn't be nullptr.

Anyway certainly room for more discussion.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D45375/new/

https://reviews.llvm.org/D45375