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

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

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

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

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));

> 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;

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

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

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

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