Bug 13775 – [REG2.067a] Broken explicit casting of dynamic array slices of known size to static array of different type

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2014-11-25T14:58:00Z
Last change time
2015-02-18T03:40:51Z
Keywords
ice, pull, rejects-valid
Assigned to
nobody
Creator
verylonglogin.reg

Comments

Comment #0 by verylonglogin.reg — 2014-11-25T14:58:53Z
This code used to compile: --- void main() { ubyte[4] ubytes; byte[2] bytes = cast(byte[2]) ubytes[0 .. 2]; } --- main.d(4): Error: e2ir: cannot cast ubytes[0..2] of type ubyte[] to type byte[2] --- In case it was an accept-invalid bug, what is the reason to disallow the feature and break user code without any deprecation message and with such unpleasant minor ICE message?
Comment #1 by hsteoh — 2014-11-25T15:38:06Z
I don't think any ICE is ever deliberate! :-) It's probably a breakage caused by fixing other parts of the compiler.
Comment #2 by issues.dlang — 2014-12-01T08:12:47Z
(In reply to hsteoh from comment #1) > I don't think any ICE is ever deliberate! :-) It's probably a breakage > caused by fixing other parts of the compiler. Yeah. ICEs are never added on purpose, and if they are, the compiler dev who did it screwed up royally, but it's not that hard to accidentally break something when changing something else (which is why we have the test suite to catch most of that stuff and the beta releases to try and catch what the test suite misses). Now, as for the code, I would have thought that it would work, and if it doesn't, and that's purposeful, I have no idea what the reason would be. Certainly, I agree that if it doesn't work, it's an annoying change, even if it has a proper error message.
Comment #3 by dlang-bugzilla — 2014-12-02T11:52:56Z
Comment #4 by schveiguy — 2014-12-02T13:23:55Z
Is this really an ICE? Usually an ICE shows a DMD source line number where the assert happened, or a seg fault. This looks more like a standard error message, although the "e2ir" is out of place. I found the error message, it doesn't look like an ICE to me, just a bad error message: https://github.com/D-Programming-Language/dmd/blob/master/src/e2ir.c#L4428 Looking through history, that message has been that way for a long time. Clearly, it's a regression, and rejects-valid, but not ICE.
Comment #5 by dlang-bugzilla — 2014-12-02T13:28:05Z
I believe this is an ICE, because such errors should be raised in the frontend, and not the glue layer. But I'm not a DMD hacker.
Comment #6 by schveiguy — 2014-12-02T13:32:56Z
Usually ICE errors are either seg-faults or marked as ICE. "Intentional" ICE, that is, those caught by the compiler asserts print the DMD source line. Look through the code just in the e2ir.c file. This clearly is not *intended* to be an ICE, even if it is technically one (and of that I'm not sure). But the error message needs to be changed regardless, the "e2ir" in the message is not helpful.
Comment #7 by verylonglogin.reg — 2014-12-02T13:39:25Z
(In reply to Steven Schveighoffer from comment #6) > Usually ICE errors are either seg-faults or marked as ICE. It has already been discussed. "e2ir" message is a minor ICE and it means an incorrect expression is encountered on converting expressions to IR.
Comment #8 by schveiguy — 2014-12-02T14:16:34Z
OK, so noted. I filed a request to make this a proper ICE: https://issues.dlang.org/show_bug.cgi?id=13810
Comment #9 by k.hara.pg — 2014-12-05T14:54:08Z
(In reply to Denis Shelomovskij from comment #0) > In case it was an accept-invalid bug, what is the reason to disallow the > feature and break user code without any deprecation message and with such > unpleasant minor ICE message? The expression cast(byte[2]) ubytes[0..2] would mean a combination of - an implicit conversion T[] to T[dim] on a slice expression with CT-known boundaries - reinterpret cast T[n] to U[m] When I implemented issue 3652, I didn't think about the case. So the code had been just accidentally accepted and and fortunately worked as you intended. Then, I made the code invalid in my pull/3984 to fix some wrong-code bug. ubyte[4] ubytes; byte[3] bytes = cast(byte[3]) ubytes[2 .. 4]; // This code had been wrongly accepted before the pull/3984. Finally, following PR will accept such reinterpret cast only when the cast is appropriate. https://github.com/D-Programming-Language/dmd/pull/4193 (In reply to Steven Schveighoffer from comment #4) > Is this really an ICE? Usually an ICE shows a DMD source line number where > the assert happened, or a seg fault. > > This looks more like a standard error message, although the "e2ir" is out of > place. > > I found the error message, it doesn't look like an ICE to me, just a bad > error message: > > https://github.com/D-Programming-Language/dmd/blob/master/src/e2ir.c#L4428 > > Looking through history, that message has been that way for a long time. > > Clearly, it's a regression, and rejects-valid, but not ICE. From historical reasons, some cast operations have been verified only by glue-layer. That's why sometimes 'e2ir' errors are reported for invalid cast operations. Essentially, all semantic errors should be handled by front-end layer. So I recognize it's a not-yet fixed compiler internal issue.
Comment #10 by schveiguy — 2014-12-05T15:53:43Z
(In reply to Kenji Hara from comment #9) > (In reply to Steven Schveighoffer from comment #4) > > Is this really an ICE? Usually an ICE shows a DMD source line number where > > the assert happened, or a seg fault. > > From historical reasons, some cast operations have been verified only by > glue-layer. That's why sometimes 'e2ir' errors are reported for invalid cast > operations. OK, I understand. Is there a reason not to make this assert though? The impression it gives is confusing to many people, including myself and Denis. See issue 13810 I added.
Comment #11 by k.hara.pg — 2014-12-05T16:09:32Z
(In reply to Steven Schveighoffer from comment #10) > OK, I understand. Is there a reason not to make this assert though? To do it, we need to handle all invalid casts in front end layer. > The impression it gives is confusing to many people, including myself and Denis. A possible quick hack is removing "e2ir" from the glue-layer errors.
Comment #12 by schveiguy — 2014-12-05T16:39:00Z
(In reply to Kenji Hara from comment #11) > (In reply to Steven Schveighoffer from comment #10) > > OK, I understand. Is there a reason not to make this assert though? > > To do it, we need to handle all invalid casts in front end layer. I get that, and I get that the fact that it is seen in the glue layer is an internal error. But on the surface, it just seems that the compiler is just rejecting (in some cases correctly) a cast. The confusing part is that the error message is telling you mainly that the cast is not correct, whether wrong or right -- the 'e2ir' message looks out of place. In previous bug reports, things like cast(int)"" printed this message. The bug report was not to say that one expected this cast to work, but that "it looks like a stray 'e2ir' message is in there." But now, we know that the 'e2ir' is a clue that the internal error exists. The end result of "fixing" it is that it's now handled in the front end, but to the user, it just looks like 'e2ir' is removed (the error message is identical otherwise). > > The impression it gives is confusing to many people, including myself and Denis. > > A possible quick hack is removing "e2ir" from the glue-layer errors. I don't want this, because then it doesn't get reported and fixed for cast rejections that *should* be rejected, but aren't handled in the right layer. It just looks like a normal error and goes unreported. What I wanted is for the error to say "ICE, please fix me," instead of giving the impression it's simply a normal error. In other words the 'e2ir' looks more like a typo than a real error message. Just make it a proper ICE, and you will get proper reports.
Comment #13 by github-bugzilla — 2014-12-06T02:41:28Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/0b77f0d79c2c7f5949804483221c3f47d2bbf101 fix Issue 13775 - Broken explicit casting of dynamic array slices of known size to static array of different type Support reinterpret-cast from a CT-known boundaries slice expression that can be implicitly typed as `T[n]` to another static array type `U[m]`, iff their sizes are same (`T[n].sizeof == U[m].sizeof`). https://github.com/D-Programming-Language/dmd/commit/0a92f21363f8fd24ef3bd293673219597d17338b Merge pull request #4193 from 9rnsr/fix13775 [REG2.067a] Issue 13775 - Broken explicit casting of dynamic array slices of known size to static array of different type
Comment #14 by pro.mathias.lang — 2015-02-12T06:21:37Z
Vibe.d is suffering from it as well: ---- case AF_INET6: ubyte[16] ip = addr_ip6.sin6_addr.s6_addr; auto ret = appender!string(); ret.reserve(40); foreach (i; 0 .. 8) { if (i > 0) ret.put(':'); ret.formattedWrite("%x", bigEndianToNative!ushort(cast(ubyte[2])ip[i*2 .. i*2+2])); } return ret.data; ---- However this code still fails with a recent compiler: source/vibe/core/net.d(181): Error: cannot cast expression ip[cast(ulong)(i * 2)..cast(ulong)(i * 2 + 2)] of type ubyte[] to ubyte[2] Tested with "v2.067-devel-932e0a5" (From today, Feb 12).
Comment #15 by sinkuupump — 2015-02-12T09:17:07Z
(In reply to Mathias LANG from comment #14) > Vibe.d is suffering from it as well: > ---- > case AF_INET6: > ubyte[16] ip = addr_ip6.sin6_addr.s6_addr; > auto ret = appender!string(); > ret.reserve(40); > foreach (i; 0 .. 8) { > if (i > 0) ret.put(':'); > ret.formattedWrite("%x", bigEndianToNative!ushort(cast(ubyte[2])ip[i*2 .. > i*2+2])); > } > return ret.data; > ---- > > However this code still fails with a recent compiler: > > source/vibe/core/net.d(181): Error: cannot cast expression ip[cast(ulong)(i > * 2)..cast(ulong)(i * 2 + 2)] of type ubyte[] to ubyte[2] > > Tested with "v2.067-devel-932e0a5" (From today, Feb 12). See also Issue 13700, which covers the case.
Comment #16 by k.hara.pg — 2015-02-12T12:41:26Z
(In reply to sinkuupump from comment #15) > (In reply to Mathias LANG from comment #14) > > However this code still fails with a recent compiler: > > > > source/vibe/core/net.d(181): Error: cannot cast expression ip[cast(ulong)(i > > * 2)..cast(ulong)(i * 2 + 2)] of type ubyte[] to ubyte[2] > > > > Tested with "v2.067-devel-932e0a5" (From today, Feb 12). > > See also Issue 13700, which covers the case. Yes, it's an enhancement case.
Comment #17 by code — 2015-02-12T13:01:52Z
(In reply to Kenji Hara from comment #16) > > See also Issue 13700, which covers the case. > > Yes, it's an enhancement case. No, it's an explicit cast and it used to compile in 2.066.1. cast(ubyte[2])ip[i*2 .. i*2+2])
Comment #18 by k.hara.pg — 2015-02-12T13:31:47Z
(In reply to Martin Nowak from comment #17) > (In reply to Kenji Hara from comment #16) > > > See also Issue 13700, which covers the case. > > > > Yes, it's an enhancement case. > > No, it's an explicit cast and it used to compile in 2.066.1. > > cast(ubyte[2])ip[i*2 .. i*2+2]) Read my comment #9. It was *accidentally* accepted in 2.066.1. In git head, casting T[] to T[n] is generally disallowed (glue layer does not support such cast operation), and it will be accepted only when the slice bounds are known at CT. Enhancement 1700 will extend the CT bounds detection, but it's not yet accepted.
Comment #19 by code — 2015-02-15T21:39:55Z
(In reply to Kenji Hara from comment #18) > Read my comment #9. It was *accidentally* accepted in 2.066.1. This code used to work even with 2.065. Maybe the difference here is that it's a slice of a static array? void bar(ubyte[2] v) { } void foo(ubyte[16] ip) { foreach (i; 0 .. 8) bar(cast(ubyte[2])ip[i*2 .. i*2+2]); }
Comment #20 by k.hara.pg — 2015-02-16T02:42:14Z
(In reply to Martin Nowak from comment #19) > (In reply to Kenji Hara from comment #18) > > Read my comment #9. It was *accidentally* accepted in 2.066.1. > > This code used to work even with 2.065. > Maybe the difference here is that it's a slice of a static array? > > void bar(ubyte[2] v) > { > } > > void foo(ubyte[16] ip) > { > foreach (i; 0 .. 8) > bar(cast(ubyte[2])ip[i*2 .. i*2+2]); > } No, it's same. Issue 3652 has been implemented since 2.063, so the accepts-invalid bug was also introduced from 2.063. With 2.062, the code reports e2ir error. test.d(8): Error: e2ir: cannot cast ip[cast(uint)(i * 2)..cast(uint)(i * 2 + 2)] of type ubyte[] to type ubyte[2u] Internal error: e2ir.c 85 In 2.067a, it has been fixed back to an invalid cast. As a note, currently most of check code for the invalid casts has been moved into front-end layer. So the reported error message is a little different. test.d(8): Error: cannot cast expression ip[cast(uint)(i * 2)..cast(uint)(i * 2 + 2)] of type ubyte[] to ubyte[2]
Comment #21 by github-bugzilla — 2015-02-18T03:40:51Z
Commits pushed to 2.067 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/0b77f0d79c2c7f5949804483221c3f47d2bbf101 fix Issue 13775 - Broken explicit casting of dynamic array slices of known size to static array of different type https://github.com/D-Programming-Language/dmd/commit/0a92f21363f8fd24ef3bd293673219597d17338b Merge pull request #4193 from 9rnsr/fix13775