[PATCH] D45195: Add --check-library-dependency to maintain compatibility with other linkers

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/Driver.cpp:934
+    case OPT_start_group:
+      ++InputFile::GroupDepth;
+      break;
----------------
MaskRay wrote:
> ruiu wrote:
> > grimar wrote:
> > > I think `GroupDepth` should not be a member of `InputFile`.
> > > It could be a local variable it seems:
> > > 
> > > ```
> > >   int32_t GroupDepth = 0;
> > >   for (auto *Arg : Args) {
> > >     switch (Arg->getOption().getUnaliasedOption().getID()) {
> > > ...
> > >     case OPT_start_group:
> > >       ++GroupDepth;
> > >       break;
> > >     case OPT_end_group:
> > >       if (GroupDepth > 0)
> > >         --GroupDepth;
> > >       else
> > >         error("stray --end-group");
> > >       break;
> > > ...
> > >     }
> > > 
> > >     // All files within the same --start-group and --end-group get the
> > >     // same group ID. Otherwise, a new file will get a new group ID.
> > >     if (!GroupDepth)
> > >       ++InputFile::CurrentGroupId;
> > >   }
> > > ```
> > We can't do that because we have to handle linker scripts as well.
> It seems gold does not support nested `--start-group --end-group`
> 
> ```
> % ld.gold -\( -\( a.o -\) -\)     
> ld.gold: fatal error: May not nest groups
> ```
Oh, I didn't know that. That means we can replace GroupDepth with a boolean variable.


================
Comment at: lld/ELF/SymbolTable.cpp:336
+//
+//   ld.lld A B --start-group C D --end-group E
+//
----------------
MaskRay wrote:
> Does `A` forms group 0 while `B` forms group 1?
You are right. I'll update this comment.


https://reviews.llvm.org/D45195