[PATCH] D22852: [ELF] - Linkerscript: implemented filename specification.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:95-96
@@ -94,1 +94,4 @@
 
+static bool fileMatches(ArrayRef<StringRef> ExcludedFiles,
+                        StringRef IncludedFiles, StringRef FileName) {
+  if (!globMatch(IncludedFiles, FileName))
----------------
ruiu wrote:
> Pass InputSectionDescription instead of the two arguments.
Done.

================
Comment at: ELF/LinkerScript.cpp:450
@@ +449,3 @@
+  void readInputSectionDescription(InputSectionDescription *InCmd, bool Keep);
+  void continueReadInputSectionDescription(InputSectionDescription *InCmd,
+                                           bool Keep);
----------------
ruiu wrote:
> This name seems odd. Everything is named readXXX where XXX is a name which the function is expected to read. This function name is not consistent.
> 
> So this function reads XXX where XXX appears
> 
>   *(XXX)
>   KEEP(*(XXX))
>   EXCLUDED_FILES(foo.o, XXX)
> 
> right? Maybe I'd call it readInputFilePattern.
No. It read everything except KEEP which I thought would be reasonable to leave at higher level as everyhing in input section description can be surrounded by keep, so readInputFilePattern would not work I think.
I refactored that place.

================
Comment at: ELF/LinkerScript.cpp:705
@@ -688,2 +704,3 @@
 
-void ScriptParser::readKeep(OutputSectionCommand *Cmd) {
+void ScriptParser::readInputSectionDescription(InputSectionDescription *InCmd,
+                                               bool Keep) {
----------------
ruiu wrote:
> This name is no longer correct. Input section description is everything that is described in https://sourceware.org/binutils/docs/ld/Input-Section.html. So it includes KEEP. You got to give it a new name.
Now it includes KEEP.

================
Comment at: ELF/LinkerScript.h:86
@@ -85,2 +85,3 @@
   static bool classof(const BaseCommand *C);
+  StringRef IncludedFiles;
   std::vector<StringRef> ExcludedFiles;
----------------
ruiu wrote:
> I'd name `FilePattern` and rename `Patterns` `SectionPatterns`.
Done.


https://reviews.llvm.org/D22852