D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 2006 - Empty array literals with explicit type implicitly convert to any array type
Summary: Empty array literals with explicit type implicitly convert to any array type
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D1 (retired)
Hardware: All All
: P2 normal
Assignee: yebblies
URL:
Keywords: pull, wrong-code
Depends on:
Blocks:
 
Reported: 2008-04-18 11:39 UTC by Bartosz Milewski
Modified: 2012-07-20 23:34 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Bartosz Milewski 2008-04-18 11:39:00 UTC
I have an empty array of arrays of string aas. I append to it an empty array of string. The result should be an array aas of length one. 

string [][] aas = [];
aas ~= cast (string []) [];
writefln ("%d", aas.length);
aas = aas ~ cast (string []) [];
writefln ("%d", aas.length);

It works correctly when using assignment and concatenation, but id doesn't when using the combined operator ~=. In the latter case, the length of array remains zero.
Comment 1 Bill Baxter 2008-04-18 14:46:18 UTC
That's funny I was just about to file this one myself a couple of days ago, BUT then I decided that it probably isn't really a bug.

[] could be an empty array of any type.  So the compiler is interpreting it as an empty string[][], not an empty string[].  You have a T[], you concat an empty T[],  your original array isn't supposed to change.

I will agree it's not the most useful of behaviors, and the doc could certainly be clearer about what will happen in such a case, but I don't think its actually a bug.  It's just the compiler interpreting [] in a not-so-useful manner.  If you type a literal "~= []" in the code then clearly you want something to happen.

The workaround is to use "~= [[]]" instead.
Comment 2 Stewart Gordon 2008-11-21 15:08:15 UTC
(In reply to comment #1)
> [] could be an empty array of any type.  So the compiler is interpreting it as
> an empty string[][], not an empty string[].  You have a T[], you concat an
> empty T[],  your original array isn't supposed to change.

But it has been explicitly cast to a string[].  How can a CastExpression be of any type other than that to which it is cast?
Comment 3 Bill Baxter 2008-12-22 19:34:25 UTC
I just got bitten by this again.

float[][] arr;
arr ~= [1.0]; // ok, adds a new element (an array of length 1).
arr ~= []; // not ok, does nothing. :-(

The last line there does nothing, apparently because the compiler interprets it to be an array of array that's empty, which is the least useful interpretation.  So I find it unexpected that the compiler interprets it this way.  Once again... even though I already ran into it once.  I just forgot because it seems so silly for the compiler to choose the interpretation that it does.

At the very least I'd like the compiler to generate an error saying it doesn't know how to interpret 'arr ~= []'.
Comment 4 Don 2009-02-19 07:06:38 UTC
Applies equally to D1.
Comment 5 Denis Derman 2011-01-28 06:39:21 UTC
(In reply to comment #3)
> I just got bitten by this again.
> 
> float[][] arr;
> arr ~= [1.0]; // ok, adds a new element (an array of length 1).
> arr ~= []; // not ok, does nothing. :-(
> 
> The last line there does nothing, apparently because the compiler interprets it
> to be an array of array that's empty, which is the least useful interpretation.
>  So I find it unexpected that the compiler interprets it this way.  Once
> again... even though I already ran into it once.  I just forgot because it
> seems so silly for the compiler to choose the interpretation that it does.
> 
> At the very least I'd like the compiler to generate an error saying it doesn't
> know how to interpret 'arr ~= []'.

Yes, ambiguity that is bug-prone, because both interpretations can run, must yield compiler error.

Denis
Comment 6 Stewart Gordon 2011-10-01 13:13:23 UTC
(In reply to comment #5)
> (In reply to comment #3)
>> At the very least I'd like the compiler to generate an error saying it doesn't
>> know how to interpret 'arr ~= []'.
> 
> Yes, ambiguity that is bug-prone, because both interpretations can run, must
> yield compiler error.

Yes, an uncast [] is ambiguous.  But the original bug report isn't about that, but about

    aas ~= cast (string []) [];

which is completely unambiguous.
Comment 8 Stewart Gordon 2012-02-03 17:10:48 UTC
The recent change to the summary line is wrong.

If they merely implicitly converted to any array type, the original code would behave correctly, because the semantic analyser always attempts an exact match before an implicit conversion match.  Clearly something else is going on.

(DMD 2.057 Win32)
----- bz2006.d -----
import std.stdio;

pragma(msg, typeof(cast(string[]) []));

void main() {
    string[][] aas = [];
    aas ~= cast(string[]) [];
    writefln("%d", aas.length);
    aas ~= (string[]).init;
    writefln("%d", aas.length);
    aas = aas ~ cast(string[]) [];
    writefln("%d", aas.length);

    auto empty = cast(string[]) [];
    pragma(msg, typeof(empty));
    aas ~= empty;
    writefln("%d", aas.length);
}
----------
C:\Users\Stewart\Documents\Programming\D\Tests\bugs>dmd bz2006.d
string[]
string[]

C:\Users\Stewart\Documents\Programming\D\Tests\bugs>bz2006
0
1
2
3
----------

Going by this, the problem seems to occur only with this specific representation of an empty array, and only when it is applied directly to ~= and not passed through a variable.

So it isn't an implicit conversion problem.  It would seem that DMD special-cases this form, but this special-casing is broken.
Comment 9 yebblies 2012-02-03 18:10:58 UTC
No, you're wrong.  I just wrote a patch to fix this bug, I think I know where the problem was and what I had to change to fix it.

eg.
struct S {}
 
void main()
{
        int[] x = cast(S[])[];
}

It has nothing to do with appending, but the fact that for T[][] the compiler has to choose to append an empty array as a T[] or a T[][], and it does this by checking implicit conversions, which pass incorrectly.

Please at least read the patch before deciding you know what the problem is.
Comment 10 Stewart Gordon 2012-02-04 06:15:31 UTC
(In reply to comment #9)
> It has nothing to do with appending, but the fact that for T[][] the compiler
> has to choose to append an empty array as a T[] or a T[][], and it does this by
> checking implicit conversions, which pass incorrectly.

cast(T[]) [] is a T[], simple as that.  So the bug is that, in this situation, for some perverted reason an implicit conversion match is chosen over an exact match.  Removing the implicit conversions stops this bug from biting, but I can imagine there being other places where it might still be an issue.

I'm finding it puzzling that changing a method in ArrayLiteralExp would alter the implicit conversions of a CastExpression.  But if it works, so be it.
Comment 11 yebblies 2012-02-04 06:29:46 UTC
(In reply to comment #10)
> cast(T[]) [] is a T[], simple as that.  So the bug is that, in this situation,
> for some perverted reason an implicit conversion match is chosen over an exact
> match.  Removing the implicit conversions stops this bug from biting, but I can
> imagine there being other places where it might still be an issue.
> 
> I'm finding it puzzling that changing a method in ArrayLiteralExp would alter
> the implicit conversions of a CastExpression.  But if it works, so be it.

Yes and no.

In CatAssignExp::semantic, the compiler calls implicitConvTo on e2 (the array literal) to see if it can convert to T[][].  ArrayLiteralExp::implicitConvTo returns a match level, and unless it returns MATCHnomatch the compiler essentially rewrites the expression as (e1 ~= cast(typeof(e1))e2).

All that works fine, the problem is in ArrayLiteralExp::implicitConvTo.
An array literal can implicitly convert to a type if the type is an array, and each element converts to the type's element type.

ArrayLiteralExp::implicitConvTo does that with something like the following (simplified):

{
level = assume perfect match
foreach(element)
   if conversion match is worse than level
       level = this match
}

Which results in the assumption never being corrected if the literal has zero elements, which makes sense because an untyped empty array literal converts to anything.

This breaks down if the array literal has been set an explicit type, which is what my patch fixes.

The way it's all done is actually quite clever, but it takes a bit of work to understand.
Comment 12 github-bugzilla 2012-07-19 00:14:06 UTC
Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/82700b3d2cf466cd69f57491a759a14e101467ac
Issue 2006 - Empty array literals with explicit type implicitly convert to anything

ArrayLiteralExp::implicitConvTo defaults to MATCHexact, and when there are no parameters it is never corrected.

https://github.com/D-Programming-Language/dmd/commit/9268d6fb1bedf9d931eadb5adf4067698b0d1e19
Merge pull request #691 from yebblies/issue2006

Issue 2006 - Empty array literals with explicit type implicitly convert to any array type
Comment 13 yebblies 2012-07-19 08:06:50 UTC
Fixed for D2 only.
Comment 14 Kenji Hara 2012-07-19 09:13:24 UTC
Pull request for D1:
https://github.com/D-Programming-Language/dmd/pull/1056