www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 2270] New: cast produces invalid arrays at runtime

reply d-bugmail puremagic.com writes:
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


--
Aug 06 2008
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270


smjg iname.com changed:

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





Oh dear, a case of issue 85 that's still pending.


--
Jan 11 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270


yebblies <yebblies gmail.com> changed:

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



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: -------
Jul 08 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270




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: -------
Jul 11 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270





 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: -------
Jul 11 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270




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: -------
Jul 11 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270






 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: -------
Jul 11 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270








 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: -------
Jul 11 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
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



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.





 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: -------
Jul 11 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270


Jonathan M Davis <jmdavisProg gmx.com> changed:

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



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: -------
Jul 11 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270




09:08:48 PDT ---


 On a semi-related but not all that related note


Interesting :)


 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:


calling opCast!(T) directly is not nearly as dangerous as using cast(T).  It is
expected that opCast does not do unsafe things.  Plus it is a regular function,
so it must obey the type system (unless of course it uses casts underneath). 
In particular, it does *not* affect const/immutable unless opCast is
specifically written to do so.

So I have no problem with opCast being called from std.conv.to.

Actually, I'm looking at std.conv.to in that pull request and see a significant
bug, because cast is so dangerous! (will file in a minute)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 11 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270




10:20:10 PDT ---


 Actually, I'm looking at std.conv.to in that pull request and see a significant
 bug, because cast is so dangerous! (will file in a minute)


Note issue 6288

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 11 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270






 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.


Indeed, it's a shame D has only one cast operator, which applies to both safe
and unsafe conversions.  Contrast C++, which has five (does dynamic_cast count
as safe?)


 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,


    int[] iarr = cast(int[]) cast(void[]) farr;

doesn't seem much more difficult to me.  Casting to void[] makes it explicit
that you want to discard the existing semantics of the byte sequence.


 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.


Indeed, ISTM probably just an oversight that (DM)D silently accepts this cast
that never does anything useful.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 11 2011
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2270


bearophile_hugs eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs eml.cc






 Indeed, it's a shame D has only one cast operator, which applies to both safe
 and unsafe conversions.  Contrast C++, which has five (does dynamic_cast count
 as safe?)


Some casts are easy enough to implement in Phobos library code, as example see
bug 5559

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