The input file for this option should contain a list of symbols, not a
list of sections, so it seems clearer to talk about laying out symbols
instead of sections. Referring to the file itself as the "symbol
ordering file" is consistent with --warn-symbol-ordering and less
ambiguous than "symbol file" (albeit slightly redundant).
Details
Diff Detail
- Repository
- rLLD LLVM Linker
- Build Status
Buildable 17471 Build 17471: arc lint + arc unit
Event Timeline
One could also read "Lay out symbols" as meaning the order of symbols in the symbol table, so I'm not sure if this is better. No strong opinions though.
It is arguable which is more accurate, "layout sections" or "layout symbols", because you can't layout symbols alone. But I agree that your new message is more intuitive and easier to understand.
LGTM but please wait for a while in case other people have different opinion.
That's fair. I'm not sure how to make that unambiguous without being super wordy.
Yeah, that's also true. Slightly unrelated to this change, but --warn-symbol-ordering is silent right now in case you forget -ffunction-sections (but of course the ordering doesn't work). Would it be reasonable to extend the warnings to cover that case? I'm thinking, for each symbol specified in the symbol ordering file, warn if that symbol's section also contains other symbols, though I don't know if that would lead to false positives and/or be reasonable cost-wise.
Also, if we're willing to go with a longer help text, I think it might be helpful to note that you must compile with -ffunction-sections -fdata-sections in order for symbol ordering to be able to work in the help text itself.
Yeah, that's also true. Slightly unrelated to this change, but --warn-symbol-ordering is silent right now in case you forget -ffunction-sections (but of course the ordering doesn't work). Would it be reasonable to extend the warnings to cover that case? I'm thinking, for each symbol specified in the symbol ordering file, warn if that symbol's section also contains other symbols, though I don't know if that would lead to false positives and/or be reasonable cost-wise.
Maybe we can warn if two or more symbols specified by a symbol ordering file belong to one section? I'm not sure if that would cause false positives, but it might be worth to try.
The new message is IMO slightly better, so I am OK with that patch.
But I am actually thinking about something like:
"Layout sections to place symbols in the order specified by symbol ordering file"
I agree that an additional warning here might be nice. We would probably only want to warn once (or possibly some other low limit) per section, not per symbol, as otherwise if you forget -ffunction-sections, you will get far too many warnings.
I agree. For whatever reason many system libraries (.a) are built without -ffunction-sections. A single warning per file (or even library) is probably OK.
I agree. "Layout symbols" sounds a bit odd, but maybe just because I have been working on a linker.
I really like this. I'll switch to it; thanks.
I'll look into the other warning as a follow-up.