www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Weird interaction of design choices: Duration + sum

reply =?UTF-8?B?THXDrXM=?= Marques <luis luismarques.eu> writes:
If you do this:

     import std.algorithm : sum;
     import core.time : Duration;

     Duration[] parts;
     auto total = parts.sum;

You'll get an error ("struct core.time.Duration member this is 
not accessible"). That's because the Duration constructor is 
private, and `sum` calls `sum(r, Unqual!Seed(0))`. BTW, not the 
most helpful of messages for beginners, maybe this could be 
phrased to say the ctor is private?

I suppose that the ctor is private due to the arbitrary nature of 
its input units (hecto-nanoseconds), even though Duration(0) is 
unambiguous. On the other hand, sum has to use Seed(0) instead of 
Seed.init because some types don't use 0 as .init, namely floats. 
The result is that you have to use range.sum(Duration.zero), or 
Duration.init.

Something here rubs me the wrong way, but I can't tell exactly 
what it is, or what can be done about it :)
Oct 13 2017
next sibling parent reply jmh530 <john.michael.hall gmail.com> writes:
On Friday, 13 October 2017 at 18:36:58 UTC, Luís Marques wrote:

 [snip]


What if it were package instead of private?
Oct 13 2017
next sibling parent Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Friday, October 13, 2017 19:11:40 jmh530 via Digitalmars-d wrote:

 On Friday, 13 October 2017 at 18:36:58 UTC, Luís Marques wrote:

 [snip]


 What if it were package instead of private?


What, Duration's constructor? Duration is in core.time, whereas sum is in
std.algorithm. They aren't in the same package on any level.

IMHO, allowing Duration's constructor to be public is just begging for
trouble, which is why it's not public. Time units should be typed, which
they are with dur and its aliases. We've tried to strip all untyped time
units from druntime and Phobos, and AFAIK, we've managed it. Allowing them
is just begging for bugs.

It sounds like that has had an annoying side effect in the case of sum, but
it's not like Duration is going to be the only type out there that doesn't
allow direct construction but can be used with sum. You just have to provide
the seed value yourself.

- Jonathan M Davis
Oct 13 2017
prev sibling parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 10/13/17 3:11 PM, jmh530 wrote:

 On Friday, 13 October 2017 at 18:36:58 UTC, Luís Marques wrote:

 [snip]


 
 What if it were package instead of private?


This wouldn't help, sum is in phobos, Duration in druntime.

The answer is that sum shouldn't be too clever. IMO, initializing to 0 
should be only done if the type's .init value doesn't match 0. Since 
only floating points and char/wchar/dchar do this, just do the Seed(0) 
for those, else use Seed.init.

-Steve
Oct 13 2017
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 10/13/17 3:24 PM, Steven Schveighoffer wrote:

 On 10/13/17 3:11 PM, jmh530 wrote:

 On Friday, 13 October 2017 at 18:36:58 UTC, Luís Marques wrote:

 [snip]


 What if it were package instead of private?


 
 This wouldn't help, sum is in phobos, Duration in druntime.
 
 The answer is that sum shouldn't be too clever. IMO, initializing to 0 
 should be only done if the type's .init value doesn't match 0. Since 
 only floating points and char/wchar/dchar do this, just do the Seed(0) 
 for those, else use Seed.init.


I take the last part back a bit. Some custom types may default to 
NaN-like values, so Seed(0) is necessary.

What should happen is that sum should test if Seed(0) is viable, and if 
not, use Seed.init.

-Steve
Oct 13 2017
next sibling parent Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Friday, October 13, 2017 15:28:03 Steven Schveighoffer via Digitalmars-d 
wrote:

 On 10/13/17 3:24 PM, Steven Schveighoffer wrote:

 On 10/13/17 3:11 PM, jmh530 wrote:

 On Friday, 13 October 2017 at 18:36:58 UTC, Luís Marques wrote:

 [snip]


 What if it were package instead of private?


 This wouldn't help, sum is in phobos, Duration in druntime.

 The answer is that sum shouldn't be too clever. IMO, initializing to 0
 should be only done if the type's .init value doesn't match 0. Since
 only floating points and char/wchar/dchar do this, just do the Seed(0)
 for those, else use Seed.init.


 I take the last part back a bit. Some custom types may default to
 NaN-like values, so Seed(0) is necessary.

 What should happen is that sum should test if Seed(0) is viable, and if
 not, use Seed.init.


The other advantage to that is that it wouldn't risk breaking anything (at
least, I don't think that it would), whereas even if Seed.init would work
for a type that accepted 0 with its constructor, the semantics might not be
the same, which could then break code if we just started using Seed.init for
those types.

- Jonathan M Davis
Oct 13 2017
prev sibling parent jmh530 <john.michael.hall gmail.com> writes:
On Friday, 13 October 2017 at 19:28:03 UTC, Steven Schveighoffer 
wrote:

 
 This wouldn't help, sum is in phobos, Duration in druntime.
 
 The answer is that sum shouldn't be too clever. IMO, 
 initializing to 0 should be only done if the type's .init 
 value doesn't match 0. Since only floating points and 
 char/wchar/dchar do this, just do the Seed(0) for those, else 
 use Seed.init.


 I take the last part back a bit. Some custom types may default 
 to NaN-like values, so Seed(0) is necessary.

 What should happen is that sum should test if Seed(0) is 
 viable, and if not, use Seed.init.

 -Steve


Ah, I see. Something like:

private enum bool canZeroConstruct(R) = is(typeof(R(0)) == R);

auto sum(R)(R r)
     if (isInputRange!R && !isInfinite!R && is(typeof(r.front + 
r.front)))
{
     import std.algorithm : sum;
     alias E = Unqual!(ElementType!R);
     static if (isFloatingPoint!E)
         alias Seed = typeof(E.init + 0.0); //biggest of 
double/real
     else
         alias Seed = typeof(r.front + r.front);
     static if (canZeroConstruct!E)
         return sum(r, Unqual!Seed(0));
     else
         return sum(r, Unqual!Seed.init);
}
Oct 13 2017
prev sibling parent Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Friday, October 13, 2017 18:36:58 Luís Marques via Digitalmars-d wrote:

 If you do this:

      import std.algorithm : sum;
      import core.time : Duration;

      Duration[] parts;
      auto total = parts.sum;

 You'll get an error ("struct core.time.Duration member this is
 not accessible"). That's because the Duration constructor is
 private, and `sum` calls `sum(r, Unqual!Seed(0))`. BTW, not the
 most helpful of messages for beginners, maybe this could be
 phrased to say the ctor is private?


Well, that's a general issue separate from sum or Duration specifically.
"Not accessible" is more general than talking about private, and it's
something that I would expect most C++ or D programmers to know, but I'm not
exactly in the best place to know what someone coming from another language
would know or find confusing. Certainly, if you feel that specifically
indicating the the problem is that the constructor is private would make for
a better error message, then feel free to create an enhancement request for
it:

https://issues.dlang.org

- Jonathan M Davis
Oct 13 2017