Bug 18172 – std.getopt should allow taking parameters by `ref` (like std.format.formattedRead), s.t. it can be used in @safe

Working on this, I've come to the realization that you have to give up most of the static argument checking that currently exists in getopt for this to work. Once you allow non-pointers to be used with the auto ref variadic parameters, it's practically impossible to tell the difference between an invalid signature and a valid one.

Actually the problem is way worse. Consider ======== string file1 = "file.dat"; string file2 = "file2.dat"; getopt(args, "file1", "info about arg", file1, "file2", file2); ======== It's actually, not practically, impossible to know if the string at the third arg is to be written to or is to be used as the help info. All of the arguments look like identical strings to the code. One possible fix is to use tuples to break the arguments into groups ======= string file1; string file2; getopt( args, tuple("arg1", "info about arg", file1), tuple("arg2", file2) ); =======

I'd suggest looking into how DIP 1000 can fix this problem without needing ref, since with DIP 1000, taking the address of a local variable isn't necessarily @system.

(In reply to Jonathan M Davis from comment #3) > I'd suggest looking into how DIP 1000 can fix this problem without needing > ref, since with DIP 1000, taking the address of a local variable isn't > necessarily @system. Despite marking the parameters as scope and using `@safe` to check for any security holes, creating a scoped pointer still yields an error ===== @safe unittest { auto args = ["prog", "--foo", "-b"]; bool foo; bool bar; scope bool* f = &foo; scope bool* b = &bar; auto rslt = getopt(args, "foo|f", "Some information about foo.", f, "bar|b", "Some help message about bar.", b); } ===== std/getopt.d(452): Error: cannot take address of local foo in @safe function __unittest_L446_C7 std/getopt.d(453): Error: cannot take address of local bar in @safe function __unittest_L446_C7

You have to compile with -dip1000, which Phobos isn't right now. If I try it locally with -dip1000, I get q.d(10): Error: scope variable f assigned to non-scope parameter _param_3 calling std.getopt.getopt!(string, string, bool*, string, string, bool*).getopt q.d(11): Error: scope variable b assigned to non-scope parameter _param_6 calling std.getopt.getopt!(string, string, bool*, string, string, bool*).getopt and I get a similar error if I take the address when passing to getopt like you normally would. getopt itself probably needs to have the parameters marked with scope to fix that. Regardless, it seems to me that either it's possible to tweak getopt to make this work with @safe in -dip1000, or it should be discussed with Walter how to improve DIP 1000 to make it work. Conceptually, I don't see any reason why getopt couldn't be made to work with scope and -dip1000 in @safe, because it doesn't escape the pointers. The question is what needs to be done to make it work.

Created attachment 1683 getopt.patch

(In reply to Jonathan M Davis from comment #5) > You have to compile with -dip1000, which Phobos isn't right now. If I try it > locally with -dip1000, I get > > q.d(10): Error: scope variable f assigned to non-scope parameter _param_3 > calling std.getopt.getopt!(string, string, bool*, string, string, > bool*).getopt > q.d(11): Error: scope variable b assigned to non-scope parameter _param_6 > calling std.getopt.getopt!(string, string, bool*, string, string, > bool*).getopt > > and I get a similar error if I take the address when passing to getopt like > you normally would. getopt itself probably needs to have the parameters > marked with scope to fix that. Regardless, it seems to me that either it's > possible to tweak getopt to make this work with @safe in -dip1000, or it > should be discussed with Walter how to improve DIP 1000 to make it work. > Conceptually, I don't see any reason why getopt couldn't be made to work > with scope and -dip1000 in @safe, because it doesn't escape the pointers. > The question is what needs to be done to make it work. I should have emphized more the fact that I had already modified getopt. I've attached my changes as a patch

(In reply to Jack Stouffer from comment #7) > I should have emphized more the fact that I had already modified getopt. > I've attached my changes as a patch Without compiling with -dip1000, nothing will be fixed, and Phobos isn't compiled with -dip1000, so the unit tests don't tell us anything. If I test your changes locally (minus the unit test changes) and compile your example in a separate program compiled with -dip1000, then I get this error: q.d(9): Error: @safe function D main cannot call @system function std.getopt.getopt!(string, string, bool*, string, string, bool*).getopt which indicates that the signature for getopt works just fine at that point, making the caller @safe. It's just that getopt's internals still need some fixing. If I slap @trusted on getopt to make it shut up, then I get linker errors about stuff in std.conv being undefined, but there are no more errors from the compiler itself. I think that that happens sometimes when some code is compiled with -dip1000 and some without, but I'm not sure. Unfortunately, Phobos as a whole does not work with -dip1000 yet, and I haven't done much with -dip1000. But from the looks of it, I'd say that it looks likely that we can fix getopt to be @safe with -dip1000 and scope without breaking the API. There may turn out to be bugs there that Walter will need to fix to get it fully working, but it looks promising. Either way, you can't update the unit tests right now to be @safe, because Phobos isn't compiled with -dip1000.

> then I get linker errors about stuff in std.conv being undefined, but there are no more errors from the compiler itself. I think that that happens sometimes when some code is compiled with -dip1000 and some without, but I'm not sure. -dip1000 is ABI-incompatible with normal D, but Walter (nor anyone else) seems to care. https://github.com/dlang/phobos/pull/5915#issuecomment-350426192

(In reply to Jack Stouffer from comment #7) > I should have emphized more the fact that I had already modified getopt. > I've attached my changes as a patch Please submit this as a Pull Request to https://github.com/dlang/phobos

(In reply to Walter Bright from comment #10) > (In reply to Jack Stouffer from comment #7) > > I should have emphized more the fact that I had already modified getopt. > > I've attached my changes as a patch > > Please submit this as a Pull Request to https://github.com/dlang/phobos It's at https://github.com/dlang/phobos/pull/6281 It's currently stalled due to a copying issue inherent in DIP1000's design that I'm planning on making a forum post on.

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/phobos/issues/10282 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB