[PATCH] D44214: Improve --warn-symbol-ordering.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 8 02:11:54 PST 2018


jhenderson added a reviewer: espindola.
jhenderson accepted this revision.
jhenderson added a subscriber: espindola.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM (with nit). Having thought about it, I agree with this comment by @espindola:

> I am OK with warning on both cases, but it feels like that would be a
>  separate warning. A non existing symbol showing up in the order file
>  probably means there is something wrong with how that files was
>  created. Two symbols that can't be independently moved because they are
>  in the same section (because of ICF or not) seems like a normal event in
>  most cases.

I also think it would be fairly simple to add - just detect if the priority was the default or not prior to changing it (though we'd need more info to get a better warning message. I don't mind this being spun off into a separate discussion.

P.S. Rafael - Can you confirm which of the two usernames you'd like us to use, please?



================
Comment at: lld/ELF/Writer.cpp:1062
 
-      if (auto *Sec = dyn_cast_or_null<InputSectionBase>(D->Section)) {
-        int &Priority = SectionOrder[cast<InputSectionBase>(Sec->Repl)];
-        Priority = std::min(Priority, Ent.Priority);
+      auto *Sec = dyn_cast_or_null<InputSectionBase>(cast<Defined>(Sym)->Section);
+      if (!Sec) {
----------------
clang-format?


https://reviews.llvm.org/D44214





More information about the llvm-commits mailing list