Details
Diff Detail
- Repository
- rLLD LLVM Linker
- Build Status
Buildable 20818 Build 20818: arc lint + arc unit
Event Timeline
- "Do not apply link-time values for dynamic relocations">;
+ "Do not apply link-time values for dynamic relocations (default)">;
| docs/ld.lld.1 | ||
|---|---|---|
| 48–49 | Ideally, a newline will be the best (which is also used in ld.bfd.1) But I don't know how to express that in groff_mdoc. A vertical bar compared with a comma is more clear here. --version, -v is the only example I find and I have updated it. | |
| docs/ld.lld.1 | ||
|---|---|---|
| 48–49 | The only way that I can see to do it is like this: diff --git a/lld/docs/ld.lld.1 b/lld/docs/ld.lld.1 index c9662856b89..d44995c842d 100644 --- a/lld/docs/ld.lld.1 +++ b/lld/docs/ld.lld.1 @@ -26,19 +26,24 @@ It accepts most of the same command line arguments and linker scripts as GNU linkers. .Pp These options are available: -.Bl -tag -width indent +.Bl -tag -width indent -compact +.Pp .It Fl -allow-multiple-definition Do not error if a symbol is defined multiple times. The first definition will be used. +.Pp .It Fl -as-needed Only set .Dv DT_NEEDED for shared libraries if used. +.Pp .It Fl -auxiliary Ns = Ns Ar value Set the .Dv DT_AUXILIARY field to the specified name. +.Pp .It Fl -Bdynamic +.It Fl -dy Link against shared libraries. .It Fl -Bstatic Do not link against shared libraries. | |
For reference I had a discussion on markup for option aliases with Ingo Schwarze and he wrote:
The standard method for achieving similar output as in the GNU ld(1) manual page goes as follows (without the comments i'm inserting/appending, of course):
The options are as follows: .Pp \" You need this if you use -compact on the next line. .Bl -tag -width Ds -compact .It Fl l Ar libName .\" No paragraph break here. .It Fl -library Ns = Ns Ar libName Add archive file ... .Pp \" You need this between items if you use -compact above. .It Fl L Ar searchdir .It Fl -library-path Ns = Ns Ar searchdir Add path ... .El .Pp \" You always need this if some text follows, even without -compact.
Then again, if the the options all fit on one output line, i don't really see the need and would rather recommend simply:
The options are as follows: .Bl -tag -width Ds .It Fl l Ar libName , Fl -library Ns = Ns Ar libName Add archive file ... .It Fl L Ar searchdir , Fl -library-path Ns = Ns Ar searchdir Add path ... .El .Pp
If you do this, always put the short option left of the long one, and decide alphabetic ordering by the short one, not by the long one.
FWIW I don't have a strong opinion on | vs newline to separate aliases, but if we are going to go with the newline -compact and .Pp approach I'd suggest making that change as a separate commit first so that the actual content changes here are easier to see/review.
FWIW I don't have a strong opinion on | vs newline to separate aliases, but if we are going to go with the newline -compact and .Pp approach I'd suggest making that change as a separate commit first so that the actual content changes here are easier to see/review.
Done
If you do this, always put the short option left of the long one, and decide alphabetic ordering by the short one, not by the long one.
I agree with this rule generally for other software but... for linker options (with a lot of short and long options for various historical heritage, single/double dashes...) I still have the tendency not to prefer unusual short options, e.g. -h is a rather uncommon alias of -soname
Similarly, I want to put --strip-debug before -S. --discard-all before -x.
| docs/ld.lld.1 | ||
|---|---|---|
| 42–43 | https://github.com/freebsd/freebsd/blob/4b26291ed887f8245f4c355f224743bfde7120f7/usr.bin/clang/lld/ld.lld.1#L70 uses this form but there are also other places preferring option=bar. If you think = is better than a space, I can change all of them (Eq options) to use =. | |
| 541 | The long option --version is used as the key here for alphabetical sorting. --version should be ordered before --version-script so their orders are swapped. | |
| docs/ld.lld.1 | ||
|---|---|---|
| 42–43 | The GNU's convention seems to always add = for long options that take arguments. I'd follow that convention, but can you leave them alone for now? I think you changed too many things in a single patch. Can you do that step by step? I'd revert all but the change to add missing options. | |
| docs/ld.lld.1 | ||
|---|---|---|
| 388–390 | Please also revert all these line-moving changes. | |
| docs/ld.lld.1 | ||
|---|---|---|
| 232 | Don't remove these options from the manual. | |
| docs/ld.lld.1 | ||
|---|---|---|
| 257 | This should be Do not fold .Ar symbol during ICF. | |
| docs/ld.lld.1 | ||
|---|---|---|
| 280–281 | Consistently add = between a long option and its argument. | |
| docs/ld.lld.1 | ||
|---|---|---|
| 257 | This is redundant. We don't say "this value" for other options that takes a "value", for example. | |
LGTM
Please create a small patch like this one next time for ease of code review. Thanks!
Add "(default") to this line just like we do for other default options.