[PATCH] D54726: [libcxx] Support generating linker script for static library

[Louis Dionne via Phabricator]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

Ship it, with named parameters. Don't wait over the holidays.



================
Comment at: libcxx/utils/gen_link_script.py:22
+                        action="store_true", default=False)
+    parser.add_argument("input", help="Path to libc++ library")
+    parser.add_argument("output", help="Path to libc++ linker script")
----------------
phosek wrote:
> ldionne wrote:
> > phosek wrote:
> > > ldionne wrote:
> > > > Please consider making these named parameters instead of positional parameters. That would have answered some of my questions before I had to ask them.
> > > I find it little awkward to use optional arguments as required arguments but I can do that if you prefer?
> > You can have required named arguments: `parser.add_argument('--foo', required=True)`. https://docs.python.org/2.7/library/argparse.html#required
> I know,  I was just pointing out that I find it a little awkward to use "optional" arguments as required arguments which is what "positional" is typically used for, but that's just my personal preference. I'm fine changing that if you prefer.
I don't understand why you say they're optional arguments.. To me, they are "named" arguments. Optional is when `required=False`, mandatory  is when `required=True`. But regardless, I do think that using named arguments will be much better because of the self documentation -- I don't want to have to read this script to figure out what the command in the CMake is doing.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54726/new/

https://reviews.llvm.org/D54726