[PATCH] D22617: [ELF] - Linkerscript: add InputSectionDescription command to LS parser.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:266
@@ -245,2 +265,3 @@
+  }
 
   // Add all other input sections, which are not listed in script.
----------------
grimar wrote:
> ruiu wrote:
> > grimar wrote:
> > > Afraid I am a bit lost in this puzzle :)
> > > Is that what you mean ?
> > > 
> > > ```
> > > static bool match(StringRef T, ArrayRef<StringRef> Pattern) {
> > >   for (StringRef S : Pattern)
> > >     if (globMatch(S, T))
> > >       return true;
> > >   return false;
> > > }
> > > ```
> > 1. I want parameters of similar functions be ordered in a consistent way. However, your new function takes [Pattern Target] although globMatch takes [Target Pattern]
> > 2. T is too short and doesn't convey it is a glob pattern rather.
> > 3. Apparently Pattern is not a right name for ArrayRef<StringRef> that doesn't contain patterns
> > 
> >   static bool match(StringRef Pattern, ArrayRef<StringRef> Arr) {
> >     for (StringRef S : Arr)
> >       if (globMatch(Pattern, S))
> >         return true;
> >     return false;
> >   }
> Ok, now I see what you meant. Sorry for my confusion and thanks a lot for all reviews !
```
// Returns true if S matches T. S can contain glob meta-characters.
// The asterisk ('*') matches zero or more characters, and the question
// mark ('?') matches one character.
bool elf::globMatch(StringRef S, StringRef T) {
  for (;;) {
    if (S.empty())
      return T.empty();
    if (S[0] == '*') {
      S = S.substr(1);
      if (S.empty())
        // Fast path. If a pattern is '*', it matches anything.
        return true;
      for (size_t I = 0, E = T.size(); I < E; ++I)
        if (globMatch(S, T.substr(I)))
          return true;
      return false;
    }
    if (T.empty() || (S[0] != T[0] && S[0] != '?'))
      return false;
    S = S.substr(1);
    T = T.substr(1);
  }
}
```

1) According to comments S is pattern. So it takes [Pattern, Target].
2) My function searches Name in a list of wildcards, that mean second is a list of patterns, so ArrayRef<StringRef> *contains* patterns. 

For function to work as I need, it needs to be:

```
if (globMatch(S, Pattern))
```

instead of 
```
if (globMatch(Pattern, S))
```

And that is what inconsistent with globMatch,
which has Pattern as first argument.

I am really not trying to argue here. I`ll commit it and just please adjust the naming as you want after that.


https://reviews.llvm.org/D22617