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

           Summary: cast produces invalid arrays at runtime
           Product: D
           Version: 1.033
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: benoit@tionex.de


interface I{}
class C : I {}

C[] carray = getInstance();
I[] iarray = carray; // compile error
I[] iarray = cast(I[])carray; // runtime error (1)

// correct way:
I[] iarray = new I[carray.length];
foreach( idx, c; carray ){
    iarray[idx] = c; // implicit cast
}

I use a template for doing this, but this looks so ugly.
I[] iarray = arraycast!(I)(carray);

I think the D compiler should call a runtime method in (1) to do the cast in a loop, instead of doing a simple type change that is not working correctly.

Alternatively the compiler can forbid the use of cast in cases where the result
will not more work.
The user has always the option to go over a void* cast, which is than clearly
marked as dangerous code line.

Discussion in NG: http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=74614


-- 

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


smjg@iname.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |smjg@iname.com
           Keywords|                            |wrong-code




------- Comment #1 from smjg@iname.com  2009-01-11 16:17 -------
Oh dear, a case of issue 85 that's still pending.


-- 

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


yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yebblies@gmail.com


--- Comment #2 from yebblies <yebblies@gmail.com> 2011-07-09 14:08:13 EST ---
I don't think this is a valid bug.  Casting an array bypasses all conversion checks.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

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



--- Comment #3 from Stewart Gordon <smjg@iname.com> 2011-07-11 04:21:01 PDT ---
I disagree.  The primary purpose of casting is simply to force an expression to a specified type.  This cast may or may not be legal.  For example, you can't cast between different class and struct types arbitrarily, or cast an array reference directly to a long (although, on 32-bit systems, they are the same size).

It's true that some casts are unsafe, such as casting away constancy.  But this is something one might actually want to do for various reasons (such as interfacing some foreign APIs), and the behaviour is documented (though it would be better if the means were explicit).

On the other hand, I can't see anybody wanting what actually happens when you
try casting an object array to an interface array.  And it's an easy mistake to
make, either
- by someone who hasn't yet learned that they aren't compatible
- by forgetting whether something's a class or an interface
- when trying in vain to do generic programming stuff that will work with
either classes or interfaces

Moreover, unless I'm mistaken there's no indication in the spec that it's legal but dangerous.

As such, it should either be illegal, or convert at runtime.  The drawback of the latter is that it would have to reallocate in order to perform the conversion, which would cause further confusion in that carray and iarray point to different areas of memory, contrary to the usual behaviour of array casting.

But I suppose what could be done is to define

    iarray[] = carray;

to convert each element of carray on copy, so that it works.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

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



--- Comment #4 from Stewart Gordon <smjg@iname.com> 2011-07-11 04:28:28 PDT ---
> On the other hand, I can't see anybody wanting what actually happens when you
> try casting an object array to an interface array.  And it's an easy mistake to
> make, either
> - by someone who hasn't yet learned that they aren't compatible
> - by forgetting whether something's a class or an interface
> - when trying in vain to do generic programming stuff that will work with
> either classes or interfaces

Add to that: when a library template that was only designed to work with classes contains such code, and somebody tries to use it with an interface.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

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



--- Comment #5 from yebblies <yebblies@gmail.com> 2011-07-11 23:36:00 EST ---
I do understand, but nothing you've said changes the fact that the behavior is completely intentional.  Cast is, as always, a blunt instrument that bypasses the type system.  Another example would be casting int to float vs casting int[] to float[].  The compiler's behavior here is as intended and consistent.

Since 2008, when this bug was reported, some things have improved.  This cast is not allowed in @safe code and std.conv.to is quite capable of performing the element by element conversion.  I don't see introducing an implicit allocation and a loop as the solution when the safe way is already supported.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

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



--- Comment #6 from Stewart Gordon <smjg@iname.com> 2011-07-11 06:45:20 PDT ---
(In reply to comment #5)
> I do understand, but nothing you've said changes the fact that the behavior is completely intentional.

What is your source for this "fact"?

> Cast is, as always, a blunt instrument that bypasses the type system.

No it isn't.  That's what I've just been explaining.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

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



--- Comment #7 from yebblies <yebblies@gmail.com> 2011-07-12 00:38:32 EST ---
(In reply to comment #6)
> (In reply to comment #5)
> > I do understand, but nothing you've said changes the fact that the behavior is completely intentional.
> 
> What is your source for this "fact"?
> 

Conversations on the newsgroup, where array casting has been discussed.  If I could find a post I'd link it, but I really have no intention of spending any more of my time searching.  I guess we can wait until the spec is updated to actually reflect what dmd does.

> > Cast is, as always, a blunt instrument that bypasses the type system.
> 
> No it isn't.  That's what I've just been explaining.

I guess we'll just have to agree to disagree then.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

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


Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|wrong-code                  |
                 CC|                            |schveiguy@yahoo.com
           Severity|normal                      |enhancement


--- Comment #8 from Steven Schveighoffer <schveiguy@yahoo.com> 2011-07-11 08:05:14 PDT ---
Casting is for all intents and purposes a blunt instrument.  The fact that cast can call runtime functions or member functions in certain situations somewhat muddies the waters.

IMO, cast should be reserved for specifically "I know what I'm doing" stuff. dynamic cast (casting from a base to derived class) should really be handled by a library function, as it is so dangerous to use cast in this case (you can unintentionally remove const or immutable).  Same thing for opCast.

For me, the litmus test for this is:

float f = 1.5;
int i = cast(int)f; // now contains 1

float[] farr = [1.5, 2.5];
int[] iarr = cast(int[])farr;

is it reasonable for someone to expect iarr to contain the binary float representation of [1.5, 2.5], or should it contain [1, 2]?

I think both cases are reasonable, and the former is much more difficult to do a different way, whereas a library function already exists to do the latter (std.conv.to).  It would be extremely inconsistent for arrays to cast differently depending on the element type.  Remember that arrays are akin to pointers, and should cast similarly (casting a pointer does nothing except reinterpret the type).

The final thing is, cast should *not* be doing hidden allocations.  I think this bug is really an enhancement request, because the current code works as designed.  Marking as such.

(In reply to comment #6)
> (In reply to comment #5)
> > I do understand, but nothing you've said changes the fact that the behavior is completely intentional.
> 
> What is your source for this "fact"?

http://www.digitalmars.com/d/2.0/expression.html#CastExpression

Note the lack of special case (and there are several) for casting arrays.  In fact, mention of array casting should be made there, to indicate how the length member is affected.  I'd throw in pointer casting as well.

So I'd interpret that as the D spec and the compiler was intentionally *not* designed to special case casting of arrays of classes to arrays of interfaces and vice versa.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

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


Jonathan M Davis <jmdavisProg@gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg@gmx.com


--- Comment #9 from Jonathan M Davis <jmdavisProg@gmx.com> 2011-07-11 08:58:04 PDT ---
On a semi-related but not all that related note, I would point out to Steven that Kenji has made pull requests related to std.conv.to, one of which will make std.conv.to use opCast for user-defined conversions if a user-defined type has an opCast, so if you don't think that opCast should be used for such situations (that is, if you equate that to dangerous as you've been discussing here), then you should comment on the pull request:

https://github.com/D-Programming-Language/phobos/pull/118 https://github.com/D-Programming-Language/phobos/pull/119 https://github.com/D-Programming-Language/phobos/pull/122

If no one else comments on those changes soon, they're likely to go in soon. But they're giving a way to avoid having to use casts in your own code, so I would think that they'd be an improvement, because then casts are used _less_ rather than more.

In any case, as for this particular bug, casts are definitely a blunt instrument, so they tend to be legal whether you really want them to or not. They're not quite as blunt as they are in C (e.g. they _do_ do checks when converting between classes), but they're still pretty blunt. It's one of the reasons that it's typically better to use std.conv.to rather than casting.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------