This is an archive of the discontinued LLVM Phabricator instance.

[llvm-nm] Don't error out on multiple occurrances of the -g/--external-only flag
ClosedPublic

Authored by mstorsjo on Nov 2 2017, 2:31 AM.

Details

Summary

GNU binutils nm doesn't error out on this, and some projects' build systems can end up doing that in some cases. Allowing that seems like a better target than trying to avoid user projects passing multiple -g parameters to $NM.

Diff Detail

Event Timeline

mstorsjo created this revision.Nov 2 2017, 2:31 AM
smeenai requested changes to this revision.Nov 2 2017, 9:08 AM

I'm not super familiar with the option parser by any means, but I think changing specific flags to be ZeroOrMore rather than Optional is much better than doing a global change for the behavior of Optional.

This revision now requires changes to proceed.Nov 2 2017, 9:08 AM
zturner edited edge metadata.Nov 2 2017, 9:15 AM

Agreed. Optional should basically be called OneOrZero. And if you think about it in those terms, it obviously doesn't make sense to allow it to be specified more than once.

Just change this specific option to be ZeroOrMore

Agreed. Optional should basically be called OneOrZero. And if you think about it in those terms, it obviously doesn't make sense to allow it to be specified more than once.

Just change this specific option to be ZeroOrMore

Right - I hadn't realized how one could change that in cl::opt - now I see.

mstorsjo updated this revision to Diff 121358.Nov 2 2017, 1:31 PM
mstorsjo edited edge metadata.
mstorsjo retitled this revision from [Support] Don't error out on multiple occurrances of an Optional flag to [llvm-nm] Don't error out on multiple occurrances of the -g/--external-only flag.
mstorsjo edited the summary of this revision. (Show Details)

Changed the type of this option instead.

ruiu accepted this revision.Nov 2 2017, 1:34 PM

LGTM

This revision was automatically updated to reflect the committed changes.