www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Rant: Date and Time fall short of simplicity in D

reply Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
import core.thread;
import core.time;
import std.datetime;
import std.stdio;

void main()
{
    StopWatch sw;
    sw.start();
    Thread.sleep(dur!"seconds"(1));
    Thread.sleep(dur!"msecs"(200));
    Thread.sleep(dur!"usecs"(800));
    sw.stop();

    TickDuration time = sw.peek();

    long secs = time.seconds;
    long msecs = time.msecs - (1000 * secs);
    long usecs = time.usecs - (1000 * msecs) - (1000 * 1000 * secs);

    writefln("secs: %s, msecs: %s, usecs: %s", secs, msecs, usecs);
}

The above works, but it's really messy having to do this all by hand.
So the other workaround is:

    TickDuration time = sw.peek();
    long secs = time.seconds;
    time -= cast(TickDuration)dur!"seconds"(secs);

    long msecs = time.msecs;
    time -= cast(TickDuration)dur!"msecs"(msecs);

    long usecs = time.usecs;

Note that the casts are necessary since there is no "tickdur!()"
function and a TickDuration cannot be subtracted with a Duration (I
don't understand why it's implemented like this). The whole date and
time API is extremely non orthogonal and ugly in D.

For example, Duration has seconds, msecs, usecs as property functions
but they are all calculated as the duration minus the larger units, so
1.200 seconds means that time.seconds == 1 && time.msecs == 200. But
TickDuration uses a different API where time.seconds == 1 &&
time.msecs == 1200.

Why was this inconsistency introduced?

--

The documentation for various property functions was clearly
copy-pasted, take a look for example at these:

    /++
        The value of this $(D FracSec) as milliseconds.
      +/
     property int msecs()  safe const pure nothrow;

    /++
        The value of this $(D FracSec) as milliseconds.

        Params:
            milliseconds = The number of milliseconds passed the second.

        Throws:
            $(D TimeException) if the given value is not less than $(D 1) second
            and greater than a $(D -1) seconds.
      +/
     property void msecs(int milliseconds)  safe pure;

The second property function is a setter, it should be documented as:

"Sets the value of this $(D FracSec) to $(B milliseconds) number of
milliseconds."

I've seen this type of property documentation copied everywhere in
date/time-related structures and classes, it's not clear from the
documentation that these are setters, they should be documented as
such.

---

Another example, I once had to convert a long type which represented
Unix time into DateTime. Here's the code to do it:

return cast(DateTime)SysTime(unixTimeToStdTime(cast(int)d.when.time));

It's unbelievable how ugly this is, and it took me way over half an
hour searching the docs and trying various stuff out to figure this
out.

---

Anyway, maybe time and datetime are power-houses in Druntime and
Phobos, but they trade their features for simplicity.

Perhaps the real problem is the documentation, or the actual layout of
the API itself. The API seems to contain a ton of functionality, and
maybe the more specialized functions should be moved into separate
modules (and make datetime be part of its own package).

The docs for std.datetime for example are huge.
Mar 28 2013
next sibling parent "Kagamin" <spam here.lot> writes:
There are many many time formats. Do you want them all readily 
supported?
Mar 28 2013
prev sibling next sibling parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Fri, 29 Mar 2013 01:42:41 -0400, Andrej Mitrovic  
<andrej.mitrovich gmail.com> wrote:


 import core.thread;
 import core.time;
 import std.datetime;
 import std.stdio;

 void main()
 {
     StopWatch sw;
     sw.start();
     Thread.sleep(dur!"seconds"(1));
     Thread.sleep(dur!"msecs"(200));
     Thread.sleep(dur!"usecs"(800));


Thread.sleep(1.seconds + 200.msecs + 800.usecs);


     sw.stop();

     TickDuration time = sw.peek();

     long secs = time.seconds;
     long msecs = time.msecs - (1000 * secs);
     long usecs = time.usecs - (1000 * msecs) - (1000 * 1000 * secs);

     writefln("secs: %s, msecs: %s, usecs: %s", secs, msecs, usecs);
 }


This one is VERY annoying, Duration has some of the necessary properties.   
The sub-second values you must obtain with FracSecs, and once you get  
there, it does NOT give you properties which remove the higher units.

So I can't really do this any better than you did.

core.time needs to be fixed.  Duration is really what you should use when  
doing duration math.  I don't know why TickDuration still exists actually.


 Why was this inconsistency introduced?


Well, Duration was the basic type for general date time stuff that  
Jonathan came up with.  TickDuration was created for StopWatch, which was  
developed by Kato Shoichi.

IMO, they should be combined.


 Another example, I once had to convert a long type which represented
 Unix time into DateTime. Here's the code to do it:

 return cast(DateTime)SysTime(unixTimeToStdTime(cast(int)d.when.time));


I have three comments here:

1. unixTimeToStdTime should take ulong.
2. There should be a shortcut for this.

Note on Windows, given a SYSTEMTIME we can do:

return cast(DateTime)SYSTEMTIMEToSysTime(t);

We need an equivalent unixTimeToSysTime, and in fact, I think we can get  
rid of unixTimeToStdTime, what is the point of that?

3. I HATE "safe" cast conversions.  If you want to make a conversion, use  
a method/property.  I don't even know why D allows overloading casting.   
Casts are way too blunt for this.

The code should be:

return unixTimeToSysTime(d.when.time).asDateTime;


 Anyway, maybe time and datetime are power-houses in Druntime and
 Phobos, but they trade their features for simplicity.

 Perhaps the real problem is the documentation, or the actual layout of
 the API itself. The API seems to contain a ton of functionality, and
 maybe the more specialized functions should be moved into separate
 modules (and make datetime be part of its own package).

 The docs for std.datetime for example are huge.


This is an issue with the doc generator.  Date and time are surprisingly  
complex features, you need a lot of power to do them properly.

The doc generator should split up the docs into docs for each type.

For a better experience, see here:

http://vibed.org/temp/d-programming-language.org/phobos/std/datetime.html

-Steve
Mar 29 2013
next sibling parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 03/29/2013 03:03 PM, Steven Schveighoffer wrote:

 ...

 3. I HATE "safe" cast conversions.  If you want to make a conversion,
 use a method/property.  I don't even know why D allows overloading
 casting.  Casts are way too blunt for this.
 ...


The only conceivable reason is opCast!bool, eg. for use in if conditions.
Mar 29 2013
parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 03/29/2013 03:20 PM, Timon Gehr wrote:

 On 03/29/2013 03:03 PM, Steven Schveighoffer wrote:

 ...

 3. I HATE "safe" cast conversions.  If you want to make a conversion,
 use a method/property.  I don't even know why D allows overloading
 casting.  Casts are way too blunt for this.
 ...


 The only conceivable reason is opCast!bool, eg. for use in if conditions.


(In case this is not clear, a conversion method cannot really do the job 
satisfactorily because of the if(auto x = foo()) construct.)
Mar 29 2013
parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Fri, 29 Mar 2013 10:22:43 -0400, Timon Gehr <timon.gehr gmx.ch> wrote:


 On 03/29/2013 03:20 PM, Timon Gehr wrote:

 On 03/29/2013 03:03 PM, Steven Schveighoffer wrote:

 ...

 3. I HATE "safe" cast conversions.  If you want to make a conversion,
 use a method/property.  I don't even know why D allows overloading
 casting.  Casts are way too blunt for this.
 ...


 The only conceivable reason is opCast!bool, eg. for use in if  
 conditions.


 (In case this is not clear, a conversion method cannot really do the job  
 satisfactorily because of the if(auto x = foo()) construct.)


That one I don't mind, because it is an implicit cast.  I think that  
feature actually would be better served by opBool or opTest or something  
like that.

It's the explicit casts which are horrible.  Cast makes type-checking go  
away to some degree, it's like requiring a sledge hammer to put in finish  
nails.  Plus the syntax is awkward.

-Steve
Mar 29 2013
prev sibling next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Friday, March 29, 2013 10:03:31 Steven Schveighoffer wrote:

 On Fri, 29 Mar 2013 01:42:41 -0400, Andrej Mitrovic

     sw.stop();
     
     TickDuration time = sw.peek();
     
     long secs = time.seconds;
     long msecs = time.msecs - (1000 * secs);
     long usecs = time.usecs - (1000 * msecs) - (1000 * 1000 * secs);
     
     writefln("secs: %s, msecs: %s, usecs: %s", secs, msecs, usecs);
 
 }


 
 This one is VERY annoying, Duration has some of the necessary properties.
 The sub-second values you must obtain with FracSecs, and once you get
 there, it does NOT give you properties which remove the higher units.
 
 So I can't really do this any better than you did.


Clearly, I need to take another look at FracSec then.


 core.time needs to be fixed.  Duration is really what you should use when
 doing duration math.  I don't know why TickDuration still exists actually.


I tend to agree. We _do_ need a separate time type which is in system ticks 
for the monotonic clock, but the stopwatch stuff doesn't need to use it. I'd
fix 
it, but I don't know if we can do so without breaking code. I'll have to think 
about it.


 3. I HATE "safe" cast conversions.  If you want to make a conversion, use
 a method/property.  I don't even know why D allows overloading casting.
 Casts are way too blunt for this.


It's how you define coversions to work with std.conv.to, so you could do 
to!DateTime(sysTime), which is shorter and avoids the explicit cast in your 
code (though the cast still occurs within std.conv.to).

- Jonathan M Davis
Mar 29 2013
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Fri, 29 Mar 2013 15:22:19 -0400, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:


 On Friday, March 29, 2013 10:03:31 Steven Schveighoffer wrote:



 3. I HATE "safe" cast conversions.  If you want to make a conversion,  
 use
 a method/property.  I don't even know why D allows overloading casting.
 Casts are way too blunt for this.


 It's how you define coversions to work with std.conv.to, so you could do
 to!DateTime(sysTime), which is shorter and avoids the explicit cast in  
 your
 code (though the cast still occurs within std.conv.to).


This seems like a horrible round-about way of creating a method:

1. Define opCast!OtherType
2. import std.conv
3. instead of using obj.toOtherType, use to!OtherType(obj)

Do we really need to go through this mess?  I understand the point is for  
'to' to be usable in generic code, but can't to be made to call the  
correct method instead of using cast?  I don't really like to using cast  
in this case anyway.

What about something like (omitting necessary constraints in the interest  
of brevity):

T to(T, U)(U u)
{
    return mixin!("u.to" ~ T.stringof);
}

Or at least just alias the opCast to the method.

-Steve
Mar 29 2013
parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Friday, March 29, 2013 15:33:35 Steven Schveighoffer wrote:

 On Fri, 29 Mar 2013 15:22:19 -0400, Jonathan M Davis <jmdavisProg gmx.com>
 
 wrote:

 On Friday, March 29, 2013 10:03:31 Steven Schveighoffer wrote:

 3. I HATE "safe" cast conversions. If you want to make a conversion,
 use
 a method/property. I don't even know why D allows overloading casting.
 Casts are way too blunt for this.


 
 It's how you define coversions to work with std.conv.to, so you could do
 to!DateTime(sysTime), which is shorter and avoids the explicit cast in
 your
 code (though the cast still occurs within std.conv.to).


 
 This seems like a horrible round-about way of creating a method:
 
 1. Define opCast!OtherType
 2. import std.conv
 3. instead of using obj.toOtherType, use to!OtherType(obj)
 
 Do we really need to go through this mess? I understand the point is for
 'to' to be usable in generic code, but can't to be made to call the
 correct method instead of using cast? I don't really like to using cast
 in this case anyway.
 
 What about something like (omitting necessary constraints in the interest
 of brevity):
 
 T to(T, U)(U u)
 {
 return mixin!("u.to" ~ T.stringof);
 }
 
 Or at least just alias the opCast to the method.


It used to be that std.conv.to used to on the type, but it was decided to get 
rid of that in favor of using opCast. I don't remember all of the reasons for 
it though.

But std.conv.to is the standard way to convert things, and I don't see how 
changing how std.conv.to determines how to do the conversion would help us 
any. Whether there was a to function on the type or opCast really makes no 
difference if you're using std.conv.to, and if you're not, then the way that 
the language provides to covert types - casting - works.

Unless you're arguing for using something other than std.conv.to to convert 
types, I really don't see the problem, and arguably, because std.conv.to is 
really the standard way to convert stuff, it's what should be used. So, I could 
see a definite argument for using std.conv.to in code rather than opCast, but I 
don't see much point in avoiding defining opCast on types, especially if code 
is then generally using std.conv.to rather than casting directly.

- Jonathan M Davis
Mar 29 2013
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Fri, 29 Mar 2013 17:17:58 -0400, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:


 But std.conv.to is the standard way to convert things, and I don't see  
 how
 changing how std.conv.to determines how to do the conversion would help  
 us
 any. Whether there was a to function on the type or opCast really makes  
 no
 difference if you're using std.conv.to, and if you're not, then the way  
 that
 the language provides to covert types - casting - works.

 Unless you're arguing for using something other than std.conv.to to  
 convert
 types, I really don't see the problem, and arguably, because std.conv.to  
 is
 really the standard way to convert stuff, it's what should be used. So,  
 I could
 see a definite argument for using std.conv.to in code rather than  
 opCast, but I
 don't see much point in avoiding defining opCast on types, especially if  
 code
 is then generally using std.conv.to rather than casting directly.


When I say "cast(Duration)time is ugly and dangerous" you say, "use  
std.conv.to instead."  Why?

It seems you are using std.conv.to as part of the API of core.time types.   
I can't really understand the point of this.  There exists a safe and  
necessary conversion (since both provide different features) from a  
TickDuration to a Duration.  Why would that be an obscure part of the  
API?  Why would the preferable interface be to use a cast?  Why does  
std.conv.to have to be involved to get something readable that doesn't  
contain the red-flag cast operator?  Both TickDuration and Duration know  
about each other, there is no reason to make this a dangerous operation  
(and yes, casts are dangerous and should be avoided).

It looks to me like the only reason a cast was chosen over a  
property/method is *so* it will work with std.conv.to.  I contend that it  
would be better of std.conv.to was not able to convert these types than to  
have to use cast on it to get this behavior.

If std.conv.to cannot work on type-defined conversions without opCast,  
then it is poorly implemented.  There needs to be a better mechanism.

-Steve
Mar 29 2013
next sibling parent reply "Jesse Phillips" <Jessekphillips+D gmail.com> writes:
On Friday, 29 March 2013 at 21:36:37 UTC, Steven Schveighoffer 
wrote:

 (and yes, casts are dangerous and should be avoided).


I think you are applying "common knowledge" to something where it 
doesn't apply.

auto foo = cast(TickDuration) bar;

This is not unsafe, unless you claim that opCast is being 
implemented in a dangerous manner (takes a Variant and converts 
it to 32)

I'm really confused on what your argument is. It sounds like you 
don't want to!() to be able and convert user types, but if it 
does than it shouldn't be allowed to use cast.

I'm not sure what issue you are expecting to prevent by requiring 
bar.toTickDuration() instead of using cast or to!().
Mar 29 2013
parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Fri, 29 Mar 2013 18:07:38 -0400, Jesse Phillips  
<Jessekphillips+D gmail.com> wrote:


 On Friday, 29 March 2013 at 21:36:37 UTC, Steven Schveighoffer wrote:

 (and yes, casts are dangerous and should be avoided).


 I think you are applying "common knowledge" to something where it  
 doesn't apply.

 auto foo = cast(TickDuration) bar;

 This is not unsafe, unless you claim that opCast is being implemented in  
 a dangerous manner (takes a Variant and converts it to 32)


Well, when you cast, you can inadvertently remove const, shared,  
immutable, etc.  In these cases, cast is safe, but refactoring can make  
things unsafe.  There are certain cases with cast where it happily  
discards const or immutable without so much as a peep.  Changing the type  
of bar above to const TickDuration, for example, could allow code that was  
not intended to discard const to do so.

In the case of TickDuration, it is a purely-value type, so it's not an  
issue.  But something with a reference could behave badly.  If you  
accidentally end up invoking the *compiler* cast, the type system goes out  
the window.


 I'm really confused on what your argument is. It sounds like you don't  
 want to!() to be able and convert user types, but if it does than it  
 shouldn't be allowed to use cast.


No, that's not what I'm saying.  I think the *mechanism* for 'to' to  
convert types should be something other than cast.  At least it should be  
allowed.


 I'm not sure what issue you are expecting to prevent by requiring  
 bar.toTickDuration() instead of using cast or to!().


I want to *allow* bar.toTickDuration(), and if you want to use  
to!TickDuration(), that should be fine too.  But to REQUIRE  
cast(TickDuration) or importing another module to access the API in a safe  
manner should not be considered good practice.  A cast should signal a  
red-flag for code reviews, it should not be a mundane API call.

-Steve
Mar 29 2013
prev sibling parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Friday, March 29, 2013 17:36:37 Steven Schveighoffer wrote:

 On Fri, 29 Mar 2013 17:17:58 -0400, Jonathan M Davis <jmdavisProg gmx.com>
 
 wrote:

 But std.conv.to is the standard way to convert things, and I don't see
 how
 changing how std.conv.to determines how to do the conversion would help
 us
 any. Whether there was a to function on the type or opCast really makes
 no
 difference if you're using std.conv.to, and if you're not, then the way
 that
 the language provides to covert types - casting - works.
 
 Unless you're arguing for using something other than std.conv.to to
 convert
 types, I really don't see the problem, and arguably, because std.conv.to
 is
 really the standard way to convert stuff, it's what should be used. So,
 I could
 see a definite argument for using std.conv.to in code rather than
 opCast, but I
 don't see much point in avoiding defining opCast on types, especially if
 code
 is then generally using std.conv.to rather than casting directly.


 
 When I say "cast(Duration)time is ugly and dangerous" you say, "use
 std.conv.to instead." Why?
 
 It seems you are using std.conv.to as part of the API of core.time types.
 I can't really understand the point of this. There exists a safe and
 necessary conversion (since both provide different features) from a
 TickDuration to a Duration. Why would that be an obscure part of the
 API? Why would the preferable interface be to use a cast? Why does
 std.conv.to have to be involved to get something readable that doesn't
 contain the red-flag cast operator? Both TickDuration and Duration know
 about each other, there is no reason to make this a dangerous operation
 (and yes, casts are dangerous and should be avoided).
 
 It looks to me like the only reason a cast was chosen over a
 property/method is *so* it will work with std.conv.to. I contend that it
 would be better of std.conv.to was not able to convert these types than to
 have to use cast on it to get this behavior.


std.conv.to is the standard way to convert one type to another. I see no 
reason to introduce stuff specific to core.time or std.datetime to do 
conversions. It should just hook into the standard stuff for that. If 
everything uses std.conv.to for coverting between types, then you don't have 
to worry about figuring out how a particular programmer decided that their API 
should do it - be it with casts or asOtherType toOtherType or whatever. 
std.conv.to is specifically designed so that any type can hook their own 
conversions into it, and then you can just always use std.conv.to for 
converting types.


 If std.conv.to cannot work on type-defined conversions without opCast,
 then it is poorly implemented. There needs to be a better mechanism.


I don't see why. std.conv.to specifically checks for opCast, not just that it 
can cast. So, there's nothing unsafe about it. Having it look for a function 
named convert wouldn't be any safer.

The only reason I see to object to opCast being used is that it's then 
possible to use cast(Type) rather than to!Type, and if you object to casts 
being used that way, then having std.conv.to use opCast makes it more likely 
that cast(Type) will work, because people will define it on their types so that 
they'll work with std.conv.to.

But since opCast is really just syntactic sugar that allows you to use the 
cast operator, and casting rarely works on user-defined types without opCast 
anyway (aside from converting between classes in an inheritance hierarchy), I 
really don't agree that opCast is particularly dangerous. If opCast isn't 
defined, odds are the cast won't work. And if it is, then there's really no 
difference between using the cast operator and an explicit function except that 
by using the cast operator, you're plugging into the language's conversion 
mechanism syntactically, and std.conv.to will then work for your type. And if 
you prefer std.conv.to to casting, then just use std.conv.to.

But the built-in casts are restricted enough on user-defined types, that I 
really don't see any problem with using opCast on user-defined types and then 
casting, and std.conv.to goes the extra mile of only using the cast if opCast 
is explicitly defined, so it won't use any dangerous casts even if there are 
any.

- Jonathan M Davis
Mar 29 2013
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Fri, 29 Mar 2013 18:08:28 -0400, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:


 std.conv.to is the standard way to convert one type to another. I see no
 reason to introduce stuff specific to core.time or std.datetime to do
 conversions. It should just hook into the standard stuff for that. If
 everything uses std.conv.to for coverting between types, then you don't  
 have
 to worry about figuring out how a particular programmer decided that  
 their API
 should do it - be it with casts or asOtherType toOtherType or whatever.
 std.conv.to is specifically designed so that any type can hook their own
 conversions into it, and then you can just always use std.conv.to for
 converting types.


But the one doing the work is core.time.  In essence, you have locked away  
part of the API behind cast, and in order to get it out without using  
cast, you have to import another module.

opCast is just a function, it could easily be called opTo, or simply  
to(T)().


 If std.conv.to cannot work on type-defined conversions without opCast,
 then it is poorly implemented. There needs to be a better mechanism.


 I don't see why. std.conv.to specifically checks for opCast, not just  
 that it
 can cast. So, there's nothing unsafe about it. Having it look for a  
 function
 named convert wouldn't be any safer.


Right, it's not std.conv.to that is the problem, it's the fact that you  
then have to expose your type to possible arbitrary casting.  One mistake,  
or refactor, and you have have thrown away const inadvertently.

There should be a safer way to hook 'to'.


 The only reason I see to object to opCast being used is that it's then
 possible to use cast(Type) rather than to!Type, and if you object to  
 casts
 being used that way, then having std.conv.to use opCast makes it more  
 likely
 that cast(Type) will work, because people will define it on their types  
 so that
 they'll work with std.conv.to.


This is exactly my objection.  People (like the OP in this thread) don't  
think about opCast being specifically for use with std.conv.to, they just  
use it as cast(X), which can be dangerous.


 But since opCast is really just syntactic sugar that allows you to use  
 the
 cast operator, and casting rarely works on user-defined types without  
 opCast
 anyway (aside from converting between classes in an inheritance  
 hierarchy), I
 really don't agree that opCast is particularly dangerous. If opCast isn't
 defined, odds are the cast won't work. And if it is, then there's really  
 no
 difference between using the cast operator and an explicit function  
 except that
 by using the cast operator, you're plugging into the language's  
 conversion
 mechanism syntactically, and std.conv.to will then work for your type.  
 And if
 you prefer std.conv.to to casting, then just use std.conv.to.


I've already found problems with std.conv.to and arbitrary casting.  See  
this bug:

http://d.puremagic.com/issues/show_bug.cgi?id=6288

If you aren't careful, you can easily end up casting away const without  
intending to.  If phobos can get it wrong, so can average developers.


 But the built-in casts are restricted enough on user-defined types, that  
 I
 really don't see any problem with using opCast on user-defined types and  
 then
 casting, and std.conv.to goes the extra mile of only using the cast if  
 opCast
 is explicitly defined, so it won't use any dangerous casts even if there  
 are
 any.


The issue is when you think you are invoking the opCast operator, but you  
inadvertently end up casting using the compiler's type-bypassing version.   
I agree the opCast call is safe, it's that its name coincides with the  
"throw all typechecks away" operator.

I don't think to should ignore opCast, or not use it, but there should be  
a way to hook 'to' without using opCast.  And most types should prefer  
that.

-Steve
Mar 29 2013
next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Friday, March 29, 2013 22:46:27 Steven Schveighoffer wrote:

 The issue is when you think you are invoking the opCast operator, but you
 inadvertently end up casting using the compiler's type-bypassing version.
 I agree the opCast call is safe, it's that its name coincides with the
 "throw all typechecks away" operator.


I'd have to experiment to see exactly what is and isn't accepted, but in my 
experience, the compiler rarely allows casting to or from structs without 
opCast (the same with classes except for the inheritance tree). So, I really 
don't think that there's much risk of accidentally using a cast on a user-
defined type and have it use the built-in cast operator.


 I don't think to should ignore opCast, or not use it, but there should be
 a way to hook 'to' without using opCast.  And most types should prefer
 that.


I really just don't see a problem here. If opCast is defined, it's perfectly 
safe regardless of what would happen if you tried to cast without opCast being 
defined. It's also the language-defined way to do type conversions. And I
really 
don't see any need to use anything else to make std.conv.to work. By using 
opCast, there's a standard way to define type conversion, and there's a 
standard way for it to hook into std.conv.to, which seems way better to me 
than trying to support every which way that a particular programmer wants to 
try and define a conversion function. Clearly, you think that using opCast is a 
problem, but I just don't agree. It's safe; it's standard; and it works.

- Jonathan M Davis
Mar 29 2013
prev sibling next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On 3/30/13, Jonathan M Davis <jmdavisProg gmx.com> wrote:

 If opCast is defined, it's perfectly
 safe regardless of what would happen if you tried to cast without opCast
 being defined.


Casting is generally unsafe when working with reference types like
classes. For example:

import std.stdio;
final class A
{
    B opCast()
    {
        return new B;
    }
}

class B
{
    void bar() { writeln("B.bar"); }
}

void main()
{
    A a = new A;
    auto b = cast(B)a;
    b.bar();  // no problem
}

Now, remove the opCast, recompile and run and you'll get a segfault.

So a cast is not safe, but you could try using std.conv.to in user-code:

void main()
{
    A a = new A;
    auto b = to!B(a);
    b.bar();
}

This will now throw an exception at runtime. But, notice how it didn't
catch the bug at compile-time. std.conv.to might even catch it at
compile-time if it knows the class doesn't inherit from any
user-defined classes, but generally it can't avoid using a dynamic
cast in a tree hierarchy. It will look for an opCast first, and then
try to do a dynamic cast.

But this is safer and can be caught at compile-time:

import std.stdio;

final class A
{
    T toType(T)()
        if (is(T == B))
    {
        return new B;
    }
}

class B
{
    void bar() { writeln("B.bar"); }
}

T to(T, S)(S s)
    if (__traits(hasMember, S, "toType"))
{
    return s.toType!T();
}

void main()
{
    A a = new A;
    auto b = to!B(a);
    b.bar();
}

If you remove toType, you will get a compile-time error instead of a
runtime one.

The point is to 1) Avoid using casts in user code, and 2) Avoid using
std.conv.to because it's too magical (tries opCast before trying
dynamic cast, which may not be what we want).

There needs to be an explicit handshake between what the library type
provides and what the user wants, cast is just too blunt and is a
red-flag in user-code.
Mar 30 2013
prev sibling next sibling parent Artur Skawina <art.08.09 gmail.com> writes:
On 03/30/13 07:12, Jonathan M Davis wrote:

 On Friday, March 29, 2013 22:46:27 Steven Schveighoffer wrote:

 The issue is when you think you are invoking the opCast operator, but you
 inadvertently end up casting using the compiler's type-bypassing version.
 I agree the opCast call is safe, it's that its name coincides with the
 "throw all typechecks away" operator.


 
 I'd have to experiment to see exactly what is and isn't accepted, but in my 
 experience, the compiler rarely allows casting to or from structs without 
 opCast (the same with classes except for the inheritance tree). So, I really 
 don't think that there's much risk of accidentally using a cast on a user-
 defined type and have it use the built-in cast operator.
 

 I don't think to should ignore opCast, or not use it, but there should be
 a way to hook 'to' without using opCast.  And most types should prefer
 that.


 
 I really just don't see a problem here. If opCast is defined, it's perfectly 
 safe regardless of what would happen if you tried to cast without opCast being 
 defined. It's also the language-defined way to do type conversions. And I
really 
 don't see any need to use anything else to make std.conv.to work. By using 
 opCast, there's a standard way to define type conversion, and there's a 
 standard way for it to hook into std.conv.to, which seems way better to me 
 than trying to support every which way that a particular programmer wants to 
 try and define a conversion function. Clearly, you think that using opCast is
a 
 problem, but I just don't agree. It's safe; it's standard; and it works.


   struct S {
      ubyte[] p;
   }

   import std.stdio;

   void main() {
      immutable a = S(null);
      // ...
      auto b = cast(S)a;
      writeln(typeof(a.p).stringof);
      writeln(typeof(b.p).stringof);
   }

which can easily happen in generic code. And you can't tell w/o looking
at the S implementation whether it's safe or not (with an appropriate
opCast it could be).

'cast()' is special, so you can't even use a "safer" opCast like

   auto opCast(DT)() inout { return inout(DT)(this.tupleof); }
   // or, more likely, returning some templated object instance

because a different return type is not accepted. Using a method

      auto b = a.opCast!S();

avoids these problems -- you only have to provide a correct a.opCast
implementation and the compiler will then catch caller mistakes.

'cast' /is/ dangerous, separating the safe operations from the
potentially unsafe ones is desirable. Most of the bugs caused by
mistakenly dropping 'const' etc wouldn't have happened if the explicit
casts weren't there - because the compiler would have complained,
forcing the coder to consider if the conversion really is necessary
and how to handle it properly.

artur
Mar 30 2013
prev sibling parent reply "Lars T. Kyllingstad" <public kyllingen.net> writes:
On Saturday, 30 March 2013 at 02:46:27 UTC, Steven Schveighoffer 
wrote:

 On Fri, 29 Mar 2013 18:08:28 -0400, Jonathan M Davis 
 <jmdavisProg gmx.com> wrote:


 std.conv.to is the standard way to convert one type to 
 another. I see no
 reason to introduce stuff specific to core.time or 
 std.datetime to do
 conversions. It should just hook into the standard stuff for 
 that. If
 everything uses std.conv.to for coverting between types, then 
 you don't have
 to worry about figuring out how a particular programmer 
 decided that their API
 should do it - be it with casts or asOtherType toOtherType or 
 whatever.
 std.conv.to is specifically designed so that any type can hook 
 their own
 conversions into it, and then you can just always use 
 std.conv.to for
 converting types.


 But the one doing the work is core.time.  In essence, you have 
 locked away part of the API behind cast, and in order to get it 
 out without using cast, you have to import another module.

 opCast is just a function, it could easily be called opTo, or 
 simply to(T)().


Now that we have the UFCS, std.conv.to should simply be 
implemented as:

    T to(F, T)(F from)
    {
        T t;
        from.convert(t);
        return t;
    }

Then, std.conv should provide convert() functions for built-in 
types, e.g.

   void convert(wstring from, ref string to);
   void convert(long from, ref int to);
   etc.

User-defined types could define their own convert method:

   struct MyType
   {
      void convert(ref MyOtherType tgt);
   }

This is both safer and more flexible than using opCast(), since

  1. you avoid the unfortunate association with the cast operator,
  2. convert() can be virtual if desired,
  3. convert() can be called directly to modify the target 
variable in-place.

Lars
Mar 31 2013
parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Sun, 31 Mar 2013 06:00:53 -0400, Lars T. Kyllingstad  
<public kyllingen.net> wrote:


 On Saturday, 30 March 2013 at 02:46:27 UTC, Steven Schveighoffer wrote:

 On Fri, 29 Mar 2013 18:08:28 -0400, Jonathan M Davis  
 <jmdavisProg gmx.com> wrote:


 std.conv.to is the standard way to convert one type to another. I see  
 no
 reason to introduce stuff specific to core.time or std.datetime to do
 conversions. It should just hook into the standard stuff for that. If
 everything uses std.conv.to for coverting between types, then you  
 don't have
 to worry about figuring out how a particular programmer decided that  
 their API
 should do it - be it with casts or asOtherType toOtherType or whatever.
 std.conv.to is specifically designed so that any type can hook their  
 own
 conversions into it, and then you can just always use std.conv.to for
 converting types.


 But the one doing the work is core.time.  In essence, you have locked  
 away part of the API behind cast, and in order to get it out without  
 using cast, you have to import another module.

 opCast is just a function, it could easily be called opTo, or simply  
 to(T)().


 Now that we have the UFCS, std.conv.to should simply be implemented as:

     T to(F, T)(F from)
     {
         T t;
         from.convert(t);
         return t;
     }

 Then, std.conv should provide convert() functions for built-in types,  
 e.g.

    void convert(wstring from, ref string to);
    void convert(long from, ref int to);
    etc.

 User-defined types could define their own convert method:

    struct MyType
    {
       void convert(ref MyOtherType tgt);
    }

 This is both safer and more flexible than using opCast(), since

   1. you avoid the unfortunate association with the cast operator,
   2. convert() can be virtual if desired,
   3. convert() can be called directly to modify the target variable  
 in-place.


I don't think this is flexible enough.  It may not be enough to be able to  
fill in another type.

But we don't need to go through ANY of this, just change T opCast(T)() to  
T to(T)().

This way, with UFCS, a.to!B() will either call the method if available, or  
std.conv.to (assuming it's imported).

-Steve
Apr 01 2013
prev sibling next sibling parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Friday, March 29, 2013 10:03:31 Steven Schveighoffer wrote:

 1. unixTimeToStdTime should take ulong.


Why? You're converting a time_t, so unixToStdTime explicitly uses time_t. Its 
size is system-dependent.

- Jonathan M Davis
Mar 29 2013
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Fri, 29 Mar 2013 19:35:35 -0400, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:


 On Friday, March 29, 2013 10:03:31 Steven Schveighoffer wrote:

 1. unixTimeToStdTime should take ulong.


 Why? You're converting a time_t, so unixToStdTime explicitly uses  
 time_t. Its
 size is system-dependent.


Because if your unix time is stored in a ulong, for whatever reason, you  
may then have to cast to call this function.

time_t implicitly casts to ulong, no matter the system-defined size of  
time_t.  It's also more future-proof, for dates past 2038.

Is there a specific reason to disallow accepting ulong?

-Steve
Mar 29 2013
parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Friday, March 29, 2013 22:50:16 Steven Schveighoffer wrote:

 On Fri, 29 Mar 2013 19:35:35 -0400, Jonathan M Davis <jmdavisProg gmx.com>
 
 wrote:

 On Friday, March 29, 2013 10:03:31 Steven Schveighoffer wrote:

 1. unixTimeToStdTime should take ulong.


 
 Why? You're converting a time_t, so unixToStdTime explicitly uses
 time_t. Its
 size is system-dependent.


 
 Because if your unix time is stored in a ulong, for whatever reason, you
 may then have to cast to call this function.
 
 time_t implicitly casts to ulong, no matter the system-defined size of
 time_t.  It's also more future-proof, for dates past 2038.
 
 Is there a specific reason to disallow accepting ulong?


Because the whole point is that you're operating on time_t, which isn't ulong. 
Also, specifically using an unsigned type is wrong, because time_t is signed on 
some systems. So, it could be changed to long, but using long instead time_t 
will break code on 32-bit systems for SysTime's toUnixTime, and I'd expect 
unixTimeToStdTime or unixTimeToSysTime to use the same type as toUnixTime. And 
if future-proofing is the issue, then you'll need a 64-bit system anyway, 
otherwise the C stuff that you're interacting with wouldn't work correctly with 
the larger time_t values.

I can definitely see an argument for just using long and then requiring people 
to cast if they're really using time_t and are on a 32-bit system, but that 
would break code at this point. I used time_t, because the whole point was 
that you were converting to and from time_t.

- Jonathan M Davis
Mar 29 2013
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Sat, 30 Mar 2013 02:06:14 -0400, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:


 On Friday, March 29, 2013 22:50:16 Steven Schveighoffer wrote:

 On Fri, 29 Mar 2013 19:35:35 -0400, Jonathan M Davis  
 <jmdavisProg gmx.com>

 wrote:

 On Friday, March 29, 2013 10:03:31 Steven Schveighoffer wrote:

 1. unixTimeToStdTime should take ulong.


 Why? You're converting a time_t, so unixToStdTime explicitly uses
 time_t. Its
 size is system-dependent.


 Because if your unix time is stored in a ulong, for whatever reason, you
 may then have to cast to call this function.

 time_t implicitly casts to ulong, no matter the system-defined size of
 time_t.  It's also more future-proof, for dates past 2038.

 Is there a specific reason to disallow accepting ulong?


 Because the whole point is that you're operating on time_t, which isn't  
 ulong.


No, you are operating on an integer.  Unix time is defined as the number  
of seconds since 1/1/1970.  time_t is what time() returns, there is no  
requirement for unix time to ALWAYS be stored as a time_t.


 Also, specifically using an unsigned type is wrong, because time_t is  
 signed on
 some systems. So, it could be changed to long, but using long instead  
 time_t
 will break code on 32-bit systems for SysTime's toUnixTime


toUnixTime can return time_t, which can implicitly cast to long or ulong.   
It's the *from* unix time that is a problem.

I'd argue that we probably should make it return long or ulong (I think I  
did that in Tango's time code), but that is something that can be done  
later.


 and I'd expect
 unixTimeToStdTime or unixTimeToSysTime to use the same type as  
 toUnixTime.


It should accept the type that has the most utility -- long or ulong.  As  
long as it accepts time_t without any loss, it does not harm anything.


 And
 if future-proofing is the issue, then you'll need a 64-bit system anyway,
 otherwise the C stuff that you're interacting with wouldn't work  
 correctly with
 the larger time_t values.


What C stuff am I interacting with?  Unix Time <=> SysTime conversions are  
purely D code.

It won't be very long until Unix will have to tackle this (hopefully they  
don't wait until 2037).  The most likely scenario is they just increase  
the bits for time_t to 64.  D will be more ready for that with a change to  
long/ulong for unixTimeToSysTime.

-Steve
Mar 30 2013
next sibling parent "Kagamin" <spam here.lot> writes:
On Sunday, 31 March 2013 at 02:39:48 UTC, Steven Schveighoffer 
wrote:

 And
 if future-proofing is the issue, then you'll need a 64-bit 
 system anyway,
 otherwise the C stuff that you're interacting with wouldn't 
 work correctly with
 the larger time_t values.


 What C stuff am I interacting with?  Unix Time <=> SysTime 
 conversions are purely D code.

 It won't be very long until Unix will have to tackle this 
 (hopefully they don't wait until 2037).  The most likely 
 scenario is they just increase the bits for time_t to 64.  D 
 will be more ready for that with a change to long/ulong for 
 unixTimeToSysTime.


Or they may treat it heuristically. C standard says that time_t 
is implementation defined, so if you want to know which time it 
represents, you should use gmtime function which converts it to 
broken down form - year, month etc.
Mar 31 2013
prev sibling next sibling parent "Kagamin" <spam here.lot> writes:
On Sunday, 31 March 2013 at 02:39:48 UTC, Steven Schveighoffer 
wrote:

 It won't be very long until Unix will have to tackle this 
 (hopefully they don't wait until 2037).


It looks like time_t is 64-bit on 64-bit system
http://svnweb.freebsd.org/base/head/sys/x86/include/_types.h?view=markup
So the issue is probably solved by migration to 64-bit, which can 
be done before 2037.
Mar 31 2013
prev sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Saturday, March 30, 2013 22:39:49 Steven Schveighoffer wrote:

 What C stuff am I interacting with?  Unix Time <=> SysTime conversions are
 purely D code.


If you're using pure D code, then why are you using unix time at all? I would 
expect the need for unix time in pure D code to be extremely rare. Pretty much 
the whole reason that unix time is supported in the first place is because 
that's what C uses. If we'd never had to worry about C, then there shouldn't 
have been much need for it.


 It won't be very long until Unix will have to tackle this (hopefully they
 don't wait until 2037).  The most likely scenario is they just increase
 the bits for time_t to 64. 


As I understand it, that _is_ the solution. Most everything is moving to 64-
bit anyway.


 D will be more ready for that with a change to
 long/ulong for unixTimeToSysTime.


We're ready for that already with time_t. If time_t isn't 64-bit on your 
system when 2038 comes, you're screwed anyway as long as you're dealing with 
time_t.

I'm not completely against changing what unixTimeToSysTime takes, but unix 
time is very much tied to time_t, and I really don't buy that it buys us much 
to make it take a long.

- Jonathan M Davis
Mar 31 2013
prev sibling next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On 3/29/13, Steven Schveighoffer <schveiguy yahoo.com> wrote:

 Thread.sleep(1.seconds + 200.msecs + 800.usecs);


Heh, sweet UFCS! Funny thing about this is my editor thinks "seconds"
is a fractional part and is syntax-highlighting it as a floating-point
number. Oops! :p


 3. I HATE "safe" cast conversions.  If you want to make a conversion, use
 a method/property.  I don't even know why D allows overloading casting.
 Casts are way too blunt for this.


Totally agreed. Because casts can be both defined and a blunt tool you
can never be sure you're actually calling a conversion method. It
could even be removed in a library between version releases,
potentially turning your casts unsafe.
Mar 29 2013
prev sibling parent reply "Kagamin" <spam here.lot> writes:
On Friday, 29 March 2013 at 14:03:34 UTC, Steven Schveighoffer 
wrote:

 Another example, I once had to convert a long type which 
 represented
 Unix time into DateTime. Here's the code to do it:

 return 
 cast(DateTime)SysTime(unixTimeToStdTime(cast(int)d.when.time));


 I have three comments here:

 1. unixTimeToStdTime should take ulong.
 2. There should be a shortcut for this.

 Note on Windows, given a SYSTEMTIME we can do:

 return cast(DateTime)SYSTEMTIMEToSysTime(t);

 We need an equivalent unixTimeToSysTime, and in fact, I think 
 we can get rid of unixTimeToStdTime, what is the point of that?

 3. I HATE "safe" cast conversions.  If you want to make a 
 conversion, use a method/property.  I don't even know why D 
 allows overloading casting.  Casts are way too blunt for this.

 The code should be:

 return unixTimeToSysTime(d.when.time).asDateTime;


I think anything-to-anything conversion is possible.

T toTime(F,T)(F fromValue, TimeZone zone=Utc)
{
   // first make an intermediate value
   // the source type needs to support toSysTime method
   // also it should not be a primitive type
   SysTime temp = fromValue.toSysTime(zone);
   // also define an extension method for SysTime
   // for conversion to this time type
   return temp.toTime!(T);
}

unix time will need a wrapper for strong typing

struct UnixTime { int value; }
SysTime toSysTime(UnixTime fromValue, TimeZone zone=Utc)
{ return SysTime(unixTimeToStdTime(fromValue.value), zone); }

so conversion is

return UnixTime(d.when.time).toTime!DateTime;

may be it can be reduced to

return to!DateTime(UnixTime(d.when.time));
Mar 30 2013
parent "Kagamin" <spam here.lot> writes:
 unix time will need a wrapper for strong typing

 struct UnixTime { int value; }
 SysTime toSysTime(UnixTime fromValue, TimeZone zone=Utc)
 { return SysTime(unixTimeToStdTime(fromValue.value), zone); }

 so conversion is

 return UnixTime(d.when.time).toTime!DateTime;

 may be it can be reduced to

 return to!DateTime(UnixTime(d.when.time));


For primitive types one may have functions which convert directly 
to SysTime instead of wrappers, this will also enable overloading 
if someone uses wider types with the same semantics.

SysTime unixTime(time_t fromValue, TimeZone zone=Utc)
{ return SysTime(unixTimeToStdTime(fromValue), zone); }

return unixTime(d.when.time).toTime!DateTime;

strongly typed time formats with defined conversion method to and 
from SysTime will be just

FILETIME ft;
return ft.toTime!DateTime;
Mar 30 2013
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Saturday, March 30, 2013 08:18:08 Andrej Mitrovic wrote:

 On 3/30/13, Jonathan M Davis <jmdavisProg gmx.com> wrote:

 If opCast is defined, it's perfectly
 safe regardless of what would happen if you tried to cast without opCast
 being defined.


 
 Casting is generally unsafe when working with reference types like
 classes. For example:
 
 import std.stdio;
 final class A
 {
     B opCast()
     {
         return new B;
     }
 }
 
 class B
 {
     void bar() { writeln("B.bar"); }
 }
 
 void main()
 {
     A a = new A;
     auto b = cast(B)a;
     b.bar();  // no problem
 }
 
 Now, remove the opCast, recompile and run and you'll get a segfault.
 
 So a cast is not safe, but you could try using std.conv.to in user-code:
 
 void main()
 {
     A a = new A;
     auto b = to!B(a);
     b.bar();
 }
 
 This will now throw an exception at runtime. But, notice how it didn't
 catch the bug at compile-time. std.conv.to might even catch it at
 compile-time if it knows the class doesn't inherit from any
 user-defined classes, but generally it can't avoid using a dynamic
 cast in a tree hierarchy. It will look for an opCast first, and then
 try to do a dynamic cast.
 
 But this is safer and can be caught at compile-time:
 
 import std.stdio;
 
 final class A
 {
     T toType(T)()
         if (is(T == B))
     {
         return new B;
     }
 }
 
 class B
 {
     void bar() { writeln("B.bar"); }
 }
 
 T to(T, S)(S s)
     if (__traits(hasMember, S, "toType"))
 {
     return s.toType!T();
 }
 
 void main()
 {
     A a = new A;
     auto b = to!B(a);
     b.bar();
 }
 
 If you remove toType, you will get a compile-time error instead of a
 runtime one.
 
 The point is to 1) Avoid using casts in user code, and 2) Avoid using
 std.conv.to because it's too magical (tries opCast before trying
 dynamic cast, which may not be what we want).
 
 There needs to be an explicit handshake between what the library type
 provides and what the user wants, cast is just too blunt and is a
 red-flag in user-code.


Having std.conv.to use toType wouldn't help any. You'd be in exactly the same 
situation as long as std.conv.to will do dynamic cast. It's just that when the 
type didn't define the conversion function for std.conv.to, instead of not 
defining opCast, the type would not be defining toType. Safety would completely 
unaffected.

To get what you're looking for, you'd need a conversion function that only 
ever had one way to convert types, whereas std.conv.to has a whole host of 
ways to convert types. But in the case of structs, odds are that it won't work 
without opCast being defined, and classes are essentially the same except for 
the exception that you point out, so std.conv.to and opCast very nearly get 
what you want anyway. The main hangup is that it'll try dynamic casting class 
references if opCast isn't defined, and to get what you want, that would have 
to not be permitted, which obviously isn't going to happen with std.conv.to.

But I really don't buy that defining or using opCast is particularly dangerous, 
and I don't think that it's problem at all that std.conv.to uses that instead 
of toType or anything other specific conversion function, since it's explicitly 
checking that opCast is defined before it uses it (the code that does the 
dynamic cast on class references is a completely different overload). I think 
that you're concerned about something that really isn't a problem.

- Jonathan M Davis
Mar 30 2013
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Saturday, March 30, 2013 09:15:24 Artur Skawina wrote:

 On 03/30/13 07:12, Jonathan M Davis wrote:

 On Friday, March 29, 2013 22:46:27 Steven Schveighoffer wrote:

 The issue is when you think you are invoking the opCast operator, but you
 inadvertently end up casting using the compiler's type-bypassing version.
 I agree the opCast call is safe, it's that its name coincides with the
 "throw all typechecks away" operator.


 
 I'd have to experiment to see exactly what is and isn't accepted, but in
 my
 experience, the compiler rarely allows casting to or from structs without
 opCast (the same with classes except for the inheritance tree). So, I
 really don't think that there's much risk of accidentally using a cast on
 a user- defined type and have it use the built-in cast operator.
 

 I don't think to should ignore opCast, or not use it, but there should be
 a way to hook 'to' without using opCast.  And most types should prefer
 that.


 
 I really just don't see a problem here. If opCast is defined, it's
 perfectly safe regardless of what would happen if you tried to cast
 without opCast being defined. It's also the language-defined way to do
 type conversions. And I really don't see any need to use anything else to
 make std.conv.to work. By using opCast, there's a standard way to define
 type conversion, and there's a standard way for it to hook into
 std.conv.to, which seems way better to me than trying to support every
 which way that a particular programmer wants to try and define a
 conversion function. Clearly, you think that using opCast is a problem,
 but I just don't agree. It's safe; it's standard; and it works.


    struct S {
       ubyte[] p;
    }
 
    import std.stdio;
 
    void main() {
       immutable a = S(null);
       // ...
       auto b = cast(S)a;
       writeln(typeof(a.p).stringof);
       writeln(typeof(b.p).stringof);
    }
 
 which can easily happen in generic code. And you can't tell w/o looking
 at the S implementation whether it's safe or not (with an appropriate
 opCast it could be).


You're casting away immutable. You pretty much have to assume that that's 
unsafe and should never do it unless you know exactly what's going on with the 
types involved. And no opCast is involved here, so I don't see how it's even 
relevant to the discussion.


 'cast()' is special, so you can't even use a "safer" opCast like
 
    auto opCast(DT)() inout { return inout(DT)(this.tupleof); }
    // or, more likely, returning some templated object instance
 
 because a different return type is not accepted. Using a method
 
       auto b = a.opCast!S();
 
 avoids these problems -- you only have to provide a correct a.opCast
 implementation and the compiler will then catch caller mistakes.
 
 'cast' /is/ dangerous, separating the safe operations from the
 potentially unsafe ones is desirable. Most of the bugs caused by
 mistakenly dropping 'const' etc wouldn't have happened if the explicit
 casts weren't there - because the compiler would have complained,
 forcing the coder to consider if the conversion really is necessary
 and how to handle it properly.


Built-in casts are dangerous. opCast is a completely different ballgame.

- Jonathan M Davis
Mar 30 2013
prev sibling next sibling parent Artur Skawina <art.08.09 gmail.com> writes:
On 03/30/13 09:22, Jonathan M Davis wrote:

 On Saturday, March 30, 2013 09:15:24 Artur Skawina wrote:

 On 03/30/13 07:12, Jonathan M Davis wrote:

 On Friday, March 29, 2013 22:46:27 Steven Schveighoffer wrote:

 The issue is when you think you are invoking the opCast operator, but you
 inadvertently end up casting using the compiler's type-bypassing version.
 I agree the opCast call is safe, it's that its name coincides with the
 "throw all typechecks away" operator.


 I'd have to experiment to see exactly what is and isn't accepted, but in
 my
 experience, the compiler rarely allows casting to or from structs without
 opCast (the same with classes except for the inheritance tree). So, I
 really don't think that there's much risk of accidentally using a cast on
 a user- defined type and have it use the built-in cast operator.


 I don't think to should ignore opCast, or not use it, but there should be
 a way to hook 'to' without using opCast.  And most types should prefer
 that.


 I really just don't see a problem here. If opCast is defined, it's
 perfectly safe regardless of what would happen if you tried to cast
 without opCast being defined. It's also the language-defined way to do
 type conversions. And I really don't see any need to use anything else to
 make std.conv.to work. By using opCast, there's a standard way to define
 type conversion, and there's a standard way for it to hook into
 std.conv.to, which seems way better to me than trying to support every
 which way that a particular programmer wants to try and define a
 conversion function. Clearly, you think that using opCast is a problem,
 but I just don't agree. It's safe; it's standard; and it works.


    struct S {
       ubyte[] p;
    }

    import std.stdio;

    void main() {
       immutable a = S(null);
       // ...
       auto b = cast(S)a;
       writeln(typeof(a.p).stringof);
       writeln(typeof(b.p).stringof);
    }

 which can easily happen in generic code. And you can't tell w/o looking
 at the S implementation whether it's safe or not (with an appropriate
 opCast it could be).


 
 You're casting away immutable. You pretty much have to assume that that's 
 unsafe and should never do it unless you know exactly what's going on with the 
 types involved. And no opCast is involved here, so I don't see how it's even 
 relevant to the discussion.


You can't tell if there is an opCast w/o looking at S. So it's either
a perfectly fine conversion, no-op, a potentially dangerous operation, or
an error. The compiler would have been able to catch the error, but by
overloading 'cast' you've prevented it from doing that.


 Built-in casts are dangerous. opCast is a completely different ballgame.


Exactly - this is the whole point. Overloading safe conversions and
raw explicit casts introduces a kind of bugs, which wouldn't be there
otherwise.

artur
Mar 30 2013
prev sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Saturday, March 30, 2013 09:53:49 Artur Skawina wrote:

 You can't tell if there is an opCast w/o looking at S. So it's either
 a perfectly fine conversion, no-op, a potentially dangerous operation, or
 an error. The compiler would have been able to catch the error, but by
 overloading 'cast' you've prevented it from doing that.


It would only be an error because the compiler couldn't do the cast (and I 
believe that unless the memory layout is the same, casting between two structs 
without opCast doesn't work, and classes will only give an error if neither 
class is a base class of the other), so defining opCast eliminates any need for 
any error.

But regardless, if you use std.conv.to rather than casting directly, then you 
don't have to worry about whether opCast is defined or not. If it is or 
std.conv.to can convert the type in another way, then std.conv.to will work, 
and if there is no opCast and none of std.conv.to's conversions will work, 
then you'll get an error. It won't try to explicitly cast unless the type has 
defined opCast.

- Jonathan M Davis
Mar 30 2013
parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Sat, 30 Mar 2013 17:27:37 -0400, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:


 On Saturday, March 30, 2013 09:53:49 Artur Skawina wrote:

 You can't tell if there is an opCast w/o looking at S. So it's either
 a perfectly fine conversion, no-op, a potentially dangerous operation,  
 or
 an error. The compiler would have been able to catch the error, but by
 overloading 'cast' you've prevented it from doing that.


 It would only be an error because the compiler couldn't do the cast (and  
 I
 believe that unless the memory layout is the same, casting between two  
 structs
 without opCast doesn't work, and classes will only give an error if  
 neither
 class is a base class of the other), so defining opCast eliminates any  
 need for
 any error.

 But regardless, if you use std.conv.to rather than casting directly,  
 then you
 don't have to worry about whether opCast is defined or not. If it is or
 std.conv.to can convert the type in another way, then std.conv.to will  
 work,
 and if there is no opCast and none of std.conv.to's conversions will  
 work,
 then you'll get an error. It won't try to explicitly cast unless the  
 type has
 defined opCast.


You continue to miss the point.  The problem is NOT std.conv.to directly.   
The problem is that opCast can be used by typing the dangerous keyword  
cast.

It is perfectly fine that std.conv.to uses opCast, especially if it does  
so in a safe manner by calling the method directly.

BUT... if you define opCast, you ALSO define another API on your type, one  
that uses the dangerous keyword cast.  Unless there is a good reason, I  
don't think anyone should do this.  The good reason should NOT be "so it  
will work with std.conv.to".  There should be another way.  In other  
words, I want to define a way for my type to convert to some other type  
that std.conv.to can use, and NOT define it via cast.

Here is an admittedly contrived example of why it is bad:

struct S
{
    int *x;
}

struct T
{
    int *x;
    U opCast(U)() if(is(U == S)) { return S(x); }
}

This cast is perfectly safe to use.  It correctly rejects attempts to  
remove const or immutable.  Great!

Now, we define a function foo, which takes an S:

void foo(S s)
{
    *s.x = 5;
}

But, hey, why not allow any type that can CONVERT to S?

void foo(X)(X x)
{
    auto s = cast(S)x;
    *s.x = 5;
}

Now foo works with S or T, just fine!  It fails with const(T) or  
immutable(T) because opCast isn't const or immutable!

But do you see the problem here?  What happens with this?

immutable int x = 4;

auto s = immutable(S)(&x);
foo(s);

does this compile?  YES IT DOES.  And it changes the value of x to 5.   
Because 'cast' not only invokes user-defined opCast, but ALSO invokes the  
compiler's very dangerous "disregard type-safety checks" cast.  We need to  
specifically disable this version!  It's *extra* code to make it safe, and  
the author may not even realize what he did!

The indirect issue here is that std.conv.to should not be promoting using  
opCast, due to the inherent dangers of using cast.  Yes, you can define  
opCast, and yes, you can use it only via std.conv.to.  But you have now  
left a dangling hook inviting an unsuspecting coder to use cast on your  
type.  There is no reason for that, and we shouldn't have that requirement  
to hook std.conv.to.

I would argue, actually, that to(T)() should be the method that  
std.conv.to can hook.  Simply due to the advent of UFCS:

import std.conv;

a.to!B

will invoke a's specific to!B function if available, or std.conv.to!B(a)  
otherwise.   I can still use the member without importing, and generic  
code that directly invokes std.conv.to will function (just call a's to  
method).

let's not standardize on cast, let's standardize on to!  It's already the  
safer choice.

Note, we don't have to break ANY code to make this change.  We simply have  
to define 'to' in addition to types that already have opCast.  Eventually  
we may deprecate opCast on those types, but I don't see the point -- when  
people see the to function, they will use it instead of casting (we can  
also recommend not using it via documentation).  Especially if most types  
DON'T define opCast, to will simply become more natural.  You also don't  
need to understand the compiler cast rewriting to use a 'to' method.

-Steve
Mar 30 2013