[PATCH] D24758: [ELF] - Linkerscript: support complex section pattern grammar.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:159-160
@@ -158,1 +158,4 @@
 
+static void sortSections(std::vector<InputSectionData *>::iterator S,
+                         std::vector<InputSectionData *>::iterator E,
+                         SortSectionPolicy K) {
----------------
Can you pass two pointers to InputSectionData instead of iterators?

S and E? Can you rename them Begin and End if they mean begin and end?

================
Comment at: ELF/LinkerScript.cpp:197-198
@@ +196,4 @@
+    // 4. If no SORT command is given, sort according to --sort-section.
+    auto ItS = I->Sections.begin() + SizeBefore;
+    auto ItE = I->Sections.end();
+    if (Pat.SortOuter != SortSectionPolicy::None) {
----------------
  InputSectionData *Begin = I->Sections.data() + SizeBefore;
  InputSectionData *End = I->Sections.data() + I->Sections.size();

================
Comment at: ELF/LinkerScript.cpp:764-765
@@ -741,2 +763,4 @@
   Regex readFilePatterns();
-  void readSectionExcludes(InputSectionDescription *Cmd);
+  void readInputSectionsList(std::vector<SectionPattern> &Out,
+                             SortSectionPolicy K1, SortSectionPolicy K2);
+  bool readSortedInputSectionsList(std::vector<SectionPattern> &Out);
----------------
Instead of mutating an argument, create a new vector and return it. It is the common practice in this file.

================
Comment at: ELF/LinkerScript.cpp:1098-1099
@@ -1092,1 +1097,4 @@
 
+bool ScriptParser::readSortedInputSectionsList(
+    std::vector<SectionPattern> &Out) {
+  SortSectionPolicy SortOut = readSortKind();
----------------
Returns a new vector instead of mutating the argument.

================
Comment at: ELF/LinkerScript.h:104-105
@@ -102,3 +103,4 @@
 struct SectionPattern {
-  SectionPattern(llvm::Regex &&Re1, llvm::Regex &&Re2)
+  SectionPattern(llvm::Regex &&Re1, llvm::Regex &&Re2, SortSectionPolicy So1,
+                 SortSectionPolicy So2)
       : ExcludedFileRe(std::forward<llvm::Regex>(Re1)),
----------------
So1 -> Sort1

RE is a common acronym for Regular Expression, but SO is not.

================
Comment at: ELF/LinkerScript.h:119-120
@@ -113,2 +118,4 @@
   llvm::Regex SectionRe;
+  SortSectionPolicy SortOuter = SortSectionPolicy::Default;
+  SortSectionPolicy SortInner = SortSectionPolicy::Default;
 };
----------------
Remove `= ... Default`. You seem to always assign them.


https://reviews.llvm.org/D24758