[PATCH] D14140: [ELF2] SECTIONS command basic support

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:250
@@ +249,3 @@
+  InSec.InputFile = next();
+  mapBraces("(", ")", [this, &InSec]() {
+    StringRef Tok = next();
----------------
denis-protivensky wrote:
> ruiu wrote:
> > I wouldn't write this in this functional style. This can be written in a plain C style if you define skip(S) which returns true if the next token is S.
> > 
> >   expect("(");
> >   while (!skip(")") {
> >     StringRef Tok = next();
> >     if (Tok == "EXCLUDE_FILE") {
> >     ...
> `mapBraces` actually does the same. I can change `peek` to `skip`, but why to produce code duplication where it can be avoided? Procedural style may be more cumbersome here than the current functional implementation.
By duplicate, do you mean this part? I don't think this is duplicate.

  expect("(");
  while (!skip(")") {

================
Comment at: ELF/Writer.cpp:614
@@ -574,3 +613,3 @@
 
-  std::stable_sort(OutputSections.begin(), OutputSections.end(),
-                   compareOutputSections<ELFT>);
+  std::stable_sort(
+      OutputSections.begin(), OutputSections.end(),
----------------
denis-protivensky wrote:
> ruiu wrote:
> > I don't like to add more code to createSections() which is already too long. Move this lambda out of this function.
> I need `RegularSections`, which is local to the method. It's captured in the lambda, and I think it would be ugly to pass it to the comparison function along with two output sections to compare. Any ideas how to make it better?
Why do you need RegularSections in the first place? What is this !RegularSections.(A)||!RegularSections.count(B) condition for?


http://reviews.llvm.org/D14140