On Tue, 26 Feb 2013 11:09:48 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:

> On Tuesday, 26 February 2013 at 14:22:08 UTC, Steven Schveighoffer wrote:
>> On Sat, 23 Feb 2013 06:31:19 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:
>>
>>> It's been years in the coming, but we finally got it done. :)  The upshot is that the module has actually seen active use over those years, both by yours truly and others, so hopefully the worst wrinkles are already ironed out.
>>>
>>> Pull request:
>>> https://github.com/D-Programming-Language/phobos/pull/1151
>>>
>>> Code:
>>> https://github.com/kyllingstad/phobos/blob/std-process2/std/process2.d
>>>
>>> Documentation:
>>> http://www.kyllingen.net/code/std-process2/phobos-prerelease/std_process2.html
>>>
>>> I hope we can get it reviewed in time for the next release.  (The wiki page indicates that both std.benchmark and std.uni are currently being reviewed, but I fail to find any "official" review threads on the forum.  Is the wiki just out of date?)
>>
>> I just reread the docs, considering Vladimir's point about space-containing no-arg programs.  I agree there is a problem.
>>
>> We need to not get rid of the single program version of spawn, we need to simply interpret it as a no-arg program.
>>
>> To have this not work:
>>
>> spawnProcess("c:/Program Files/xyz/xyz.exe");
>>
>> and require this instead:
>>
>> spawnProcess("c:/Program Files/xyz/xyz.exe", []);
>>
>> is not very intuitive.
>>
>> It reminds me of when we had writefln and not writeln, in order to print out a string with % in it, you had to do writefln("%s", "%s");
>>
>> Now, I think we have an additional issue in that it's difficult to take a string argument with parameters in it, and pass it in one line:
>>
>> string executeThis = "prog arg1 arg2";
>> auto params = split(executeThis);
>> spawnProcess(params[0], params[1..$]);
>>
>> It would be nice to just be able to do this:
>>
>> spawnProcess(split(executeThis));
>>
>> I think we need an overload for that, especially if we get rid of the auto-splitting of commands.  It should assert if the array is empty.
>
> I propose we only have two versions:
>
> spawnProcess(string[] args, File stdin, etc...)
> spawnProcess(string[] args, string[string] env, File stdin, etc...)
>
> You'd use it like this:
>
> spawnProcess(["prog"]);

That allocates.  I don't like that requirement.  At the very least there should be version which takes a simple string, an easy thing to wrap:

auto spawnProcess(string program, File stdin, etc...)
{
   return spawnProcess((&program)[0..1], stdin, etc...);
}

We should also consider a variadic solution.  In tango, things were done with an object, so the arguments were set via one method/constructor, and the options (stdin, stdout, etc) were set via another.  This allowed the great API of

setArgs(string[] ...)

Which supports

setArgs("progname", "arg1", "arg2")

and

setArgs("progname arg1 arg2".split())

without extra allocation.  However, we have two conflicting parts to spawnProcess that would be optional -- the variadic arg list, and the optional redirected handles and configuration.

We could just go full-bore variadic...

-Steve

On Tuesday, 26 February 2013 at 16:45:08 UTC, Steven Schveighoffer wrote:
> On Tue, 26 Feb 2013 11:09:48 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:
>>
>> I propose we only have two versions:
>>
>> spawnProcess(string[] args, File stdin, etc...)
>> spawnProcess(string[] args, string[string] env, File stdin, etc...)
>>
>> You'd use it like this:
>>
>> spawnProcess(["prog"]);
>
> That allocates.  I don't like that requirement.

'scope string[] args' should tell the compiler not to allocate.


> At the very least there should be version which takes a simple string, an easy thing to wrap:
>
> auto spawnProcess(string program, File stdin, etc...)
> {
>    return spawnProcess((&program)[0..1], stdin, etc...);
> }
>
> We should also consider a variadic solution.  In tango, things were done with an object, so the arguments were set via one method/constructor, and the options (stdin, stdout, etc) were set via another.  This allowed the great API of
>
> setArgs(string[] ...)
>
> Which supports
>
> setArgs("progname", "arg1", "arg2")
>
> and
>
> setArgs("progname arg1 arg2".split())
>
> without extra allocation.  However, we have two conflicting parts to spawnProcess that would be optional -- the variadic arg list, and the optional redirected handles and configuration.
>
> We could just go full-bore variadic...

I'd rather not.

Lars

On Tuesday, 26 February 2013 at 16:45:08 UTC, Steven Schveighoffer wrote:
> On Tue, 26 Feb 2013 11:09:48 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:
>> You'd use it like this:
>>
>> spawnProcess(["prog"]);
>
> That allocates.  I don't like that requirement.

I know this is debatable and that we've discussed this before, but I feel I should still mention that the cost of one more small allocation will be absolutely negligible compared to the cost of creating a new process, even taking into account long-term effects of heap fragmentation and such. I don't think that API design should suffer for such a small performance cost.

On Tue, 26 Feb 2013 12:10:00 -0500, Vladimir Panteleev <vladimir@thecybershadow.net> wrote:

> On Tuesday, 26 February 2013 at 16:45:08 UTC, Steven Schveighoffer wrote:
>> On Tue, 26 Feb 2013 11:09:48 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:
>>> You'd use it like this:
>>>
>>> spawnProcess(["prog"]);
>>
>> That allocates.  I don't like that requirement.
>
> I know this is debatable and that we've discussed this before, but I feel I should still mention that the cost of one more small allocation will be absolutely negligible compared to the cost of creating a new process, even taking into account long-term effects of heap fragmentation and such. I don't think that API design should suffer for such a small performance cost.

Even the API is ugly.  It takes no significant code or really understanding to make a single-arg spawnProcess that does the same thing.

-Steve

On Tue, 26 Feb 2013 12:03:51 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:

> On Tuesday, 26 February 2013 at 16:45:08 UTC, Steven Schveighoffer wrote:
>> On Tue, 26 Feb 2013 11:09:48 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:

>>> spawnProcess(["prog"]);
>>
>> That allocates.  I don't like that requirement.
>
> 'scope string[] args' should tell the compiler not to allocate.

That's not how it works.  The expression [<anything>] allocates.

>> At the very least there should be version which takes a simple string, an easy thing to wrap:
>>
>> auto spawnProcess(string program, File stdin, etc...)
>> {
>>    return spawnProcess((&program)[0..1], stdin, etc...);
>> }
>>
>> We should also consider a variadic solution.  In tango, things were done with an object, so the arguments were set via one method/constructor, and the options (stdin, stdout, etc) were set via another.  This allowed the great API of
>>
>> setArgs(string[] ...)
>>
>> Which supports
>>
>> setArgs("progname", "arg1", "arg2")
>>
>> and
>>
>> setArgs("progname arg1 arg2".split())
>>
>> without extra allocation.  However, we have two conflicting parts to spawnProcess that would be optional -- the variadic arg list, and the optional redirected handles and configuration.
>>
>> We could just go full-bore variadic...
>
> I'd rather not.

Did that apply to all the statements above, or just the variadic part?

-Steve

On Tuesday, February 26, 2013 09:02:11 Steven Schveighoffer wrote:
> On one hand, I think the correct behavior is to return null, and let the program deal with checking the error, or use get if they have a default. If we throw an exception, people will end up catching the exception in order to avoid an unintended error. Exceptions are not good for flow control, they are for exceptional situations that you didn't plan for.

I think that it's far more correct to say that exceptions are for situations where it's reasonable for code to assume that something's the case when it might not be or when it's impossible for it to check. For instance, it's much cleaner to write a parser if the parser in general assumes that operations will succeed and throws when they don't. Then only a small part of the parser needs to worry about handling error cases. Or an example of when it would be impossible to check would be with file operations. You can (and should) check beforehand that the file exists, but there's no way to guarantee that the file will still exist when you actually operate on it (e.g. another process could delete it out from under you), so the file functions have to throw when the file isn't there anymore or you don't have permissions or whatever.

If you have to keep checking return values for functions, then you should probably be using exceptions. The place to avoid exceptions is when the odds of an operation succeeding are low (or at least that there's a fairly good chance that it'll fail), because then it really is just becoming flow control. But I actually think that pushing for exceptions to be for "exceptional situations" is harmful, as that leads to people not using them, and the code ends up checking return values when it would be much cleaner if it didn't have to. Of course, there are plenty of people who are quite poor at that balance and end up over-using exceptions as well, so striking a good balance can be hard.

In general though, the main question is whether it's reasonable for code to assume that an operation will succeed, and if it is, then an exception should be used. In the case of environment variables, whether that's reasonable or not depends on the code. There are programs out there which pretty much can't run if a particular environment variable hasn't been set (I deal with several at work which are that way), and if the program itself set the enviornment variable, then it should be able to assume that it's there. In either case, the exception route makes more sense. On plenty of other occasions though, it's not at all reasonable to assume that it's there and the code needs to check, in which case, throwing an exception doesn't make sense at all.

But given the fact that opIndex is already generally assumed to succeed, I think that it makes perfect sense to make it throw on failure, and then have get return null on failure. Programs can then use whichever behavior is correct for their particular needs.

- Jonathan M Davis

On Tuesday, 26 February 2013 at 18:24:08 UTC, Steven Schveighoffer wrote:
> On Tue, 26 Feb 2013 12:03:51 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:
>
>> On Tuesday, 26 February 2013 at 16:45:08 UTC, Steven Schveighoffer wrote:
>>> On Tue, 26 Feb 2013 11:09:48 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:
>
>>>> spawnProcess(["prog"]);
>>>
>>> That allocates.  I don't like that requirement.
>>
>> 'scope string[] args' should tell the compiler not to allocate.
>
> That's not how it works.  The expression [<anything>] allocates.

Isn't that just a shortcoming of DMD?  I thought 'scope' was all about avoiding such allocations.


>>> At the very least there should be version which takes a simple string, an easy thing to wrap:
>>>
>>> auto spawnProcess(string program, File stdin, etc...)
>>> {
>>>   return spawnProcess((&program)[0..1], stdin, etc...);
>>> }
>>>
>>> We should also consider a variadic solution.  In tango, things were done with an object, so the arguments were set via one method/constructor, and the options (stdin, stdout, etc) were set via another.  This allowed the great API of
>>>
>>> setArgs(string[] ...)
>>>
>>> Which supports
>>>
>>> setArgs("progname", "arg1", "arg2")
>>>
>>> and
>>>
>>> setArgs("progname arg1 arg2".split())
>>>
>>> without extra allocation.  However, we have two conflicting parts to spawnProcess that would be optional -- the variadic arg list, and the optional redirected handles and configuration.
>>>
>>> We could just go full-bore variadic...
>>
>> I'd rather not.
>
> Did that apply to all the statements above, or just the variadic part?

The variadic part.  (And the Tango-like object part, though that didn't read like a suggestion.)  A single-string version of spawnProcess() is OK with me, in combination with either of:

  spawnProcess(string, string[], etc.)
  spawnProcess(string[], etc.)

Lars

On 2013-02-26 15:50, Steven Schveighoffer wrote:

> Except there are non-string arguments at the end,
> stdin/stdout/stderr/config

Wow, that was a couple of extra parameters. Didn't actually look before.

-- 
/Jacob Carlborg

On Tuesday, February 26, 2013 20:33:14 Lars T. Kyllingstad wrote:
> On Tuesday, 26 February 2013 at 18:24:08 UTC, Steven
> 
> Schveighoffer wrote:
> > On Tue, 26 Feb 2013 12:03:51 -0500, Lars T. Kyllingstad
> > 
> > <public@kyllingen.net> wrote:
> >> On Tuesday, 26 February 2013 at 16:45:08 UTC, Steven
> >> 
> >> Schveighoffer wrote:
> >>> On Tue, 26 Feb 2013 11:09:48 -0500, Lars T. Kyllingstad
> >>> 
> >>> <public@kyllingen.net> wrote:
> >>>> spawnProcess(["prog"]);
> >>> 
> >>> That allocates. I don't like that requirement.
> >> 
> >> 'scope string[] args' should tell the compiler not to allocate.
> > 
> > That's not how it works. The expression [<anything>] allocates.
> 
> Isn't that just a shortcoming of DMD? I thought 'scope' was all about avoiding such allocations.

scope is all about enforcing that what's being passed to a function does not escape it. To quote the docs ( http://dlang.org/function.html ).

---------
ref­er­ences in the pa­ra­me­ter can­not be es­caped (e.g. as­signed to a global vari­able)
---------

That has the added benefit of allowing the compiler to make optimizations (like not allocating a closure), but scope in and of itself doesn't necessarily mean that no allocation will occur. The spec says _nothing_ on that count. It doesn't even discuss scope in relation to delegates. All that's guaranteed is that no references to parameters marked with scope will escape.

However, regardless of all that, scope is currently only implemented for delegates (and even there, it's fairly buggy IIRC), so it will have zero effect on an array.

- Jonathan M Davis

On Tue, 26 Feb 2013 14:33:14 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:

> On Tuesday, 26 February 2013 at 18:24:08 UTC, Steven Schveighoffer wrote:
>> That's not how it works.  The expression [<anything>] allocates.
>
> Isn't that just a shortcoming of DMD?  I thought 'scope' was all about avoiding such allocations.

Not that I am aware of.  Any array expression calls _d_newArray.  Even enums of array expressions call that every time they are used.

> The variadic part.  (And the Tango-like object part, though that didn't read like a suggestion.)  A single-string version of spawnProcess() is OK with me, in combination with either of:
>
>    spawnProcess(string, string[], etc.)
>    spawnProcess(string[], etc.)

OK, that sounds fine.

The tango thing was just for background (because Process was an object, you could manipulate it before executing, thus allowing the variadic setting of arguments).

-Steve