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

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 27 15:06:57 PDT 2016


ruiu 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))
----------------
Pass InputSectionDescription instead of the two arguments.

================
Comment at: ELF/LinkerScript.cpp:450
@@ +449,3 @@
+  void readInputSectionDescription(InputSectionDescription *InCmd, bool Keep);
+  void continueReadInputSectionDescription(InputSectionDescription *InCmd,
+                                           bool Keep);
----------------
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.

================
Comment at: ELF/LinkerScript.cpp:705
@@ -688,2 +704,3 @@
 
-void ScriptParser::readKeep(OutputSectionCommand *Cmd) {
+void ScriptParser::readInputSectionDescription(InputSectionDescription *InCmd,
+                                               bool Keep) {
----------------
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.

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


https://reviews.llvm.org/D22852





More information about the llvm-commits mailing list