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!().

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

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

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

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

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

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

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

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.

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.