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

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:774-775
@@ -770,2 +773,4 @@
   Regex readFilePatterns();
-  void readSectionExcludes(InputSectionDescription *Cmd);
+  std::vector<SectionPattern> readInputSectionsList(SortSectionPolicy SortOut,
+                                                    SortSectionPolicy SortIn);
+  std::vector<SectionPattern>
----------------
ruiu wrote:
> Out and In look like they are short for Output and Input, so these names are confusing. Since we know the type here, Inner and Outer should suffice.
Done.

================
Comment at: ELF/LinkerScript.cpp:1133-1137
@@ -1099,1 +1132,7 @@
+    SortSectionPolicy SortOut = readSortKind();
+    if (SortOut != SortSectionPolicy::Default)
+      V = readSortedInputSectionsList(SortOut);
+    else
+      V = readInputSectionsList(SortSectionPolicy::Default,
+                                SortSectionPolicy::Default);
 
----------------
ruiu wrote:
> This code seem too complicated compared to what it is doing. I think you could do like this.
> 
>   SortSectionPolicy Inner = readSortKind();
>   SortSectionPolicy Outer = Default;
>   if (Inner != SortSectionPolicy::Default) {
>     expect("(");
>     Outer = readSortKind();
>     if (Outer != SortSectionPolicy::Default) {
>       expect("(");
>       V = readInputSectionList();
>       expect(")");
>     } else {
>       V = readInputSectionList();
>     }
>     expect(")");
>   }
> 
>   for (SectionPattern &Pat : V) {
>     Pat.SortInner = Inner;
>     Pat.SortOuter = Outer;
>   }
> 
:) Something like this was my first idea when I strarted implementation. But then I thought you will not like the additional loop at the end..


https://reviews.llvm.org/D24758