Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REG2.067a] Issue 13775 - Broken explicit casting of dynamic array slices of known size to static array of different type #4193

Merged
merged 1 commit into from
Dec 6, 2014

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Dec 5, 2014

https://issues.dlang.org/show_bug.cgi?id=13775

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

…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`).
@Hackerpilot
Copy link
Member

I don't see any problems with this.

@yebblies
Copy link
Member

yebblies commented Dec 6, 2014

Auto-merge toggled on

yebblies added a commit that referenced this pull request Dec 6, 2014
[REG2.067a] Issue 13775 - Broken explicit casting of dynamic array slices of known size to static array of different type
@yebblies yebblies merged commit 0a92f21 into dlang:master Dec 6, 2014
@9rnsr 9rnsr deleted the fix13775 branch December 6, 2014 02:52
@CyberShadow
Copy link
Member

This code broke the following (used in ae, breaking Digger):

immutable(ubyte)[] data = new ubyte[20].idup;
auto a = cast(ubyte[20])data[0..$];

Was this breakage intentional?

@Geod24
Copy link
Member

Geod24 commented Dec 27, 2014

Thanks for tracing this @CyberShadow :)

Also affects Vibe.d:

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

@9rnsr
Copy link
Contributor Author

9rnsr commented Dec 28, 2014

Was this breakage intentional?

This PR intentionally rejects the case, because there was a wrong-code bug. See the example code:

void main() {
    immutable(ubyte)[] data = new ubyte[2].idup;
    auto a = cast(ubyte[4])data[0..$];
    // Although $ == 2, the cast will access out of the range of valid memory.

    import std.stdio;
    writeln(a);
    // will print [0, 0, garbage, garbage]
}

And, until 2.062, the reinterpret cast from T[] to T[n] had been constantly disallowed. So the restriction is merely enabled again.

@CyberShadow
Copy link
Member

IMHO this should be a runtime check, like with slice copies. So it should throw Array lengths don't match for copy: 4 != 2

@9rnsr
Copy link
Contributor Author

9rnsr commented Dec 28, 2014

Of course it's possible, but at least it should be a new enhancement. And the cast would not work under the @nogc semantics so the thrown Error would need allocation.

@CyberShadow
Copy link
Member

And the cast would not work under the @nogc semantics so the thrown Error would need allocation.

I thought asserts were exempt from @nogc? At least, you can put assert(false) in a @nogc function.

@9rnsr
Copy link
Contributor Author

9rnsr commented Dec 28, 2014

I don't know. At least today range violation handling is not same with assert(false) even in @nogc function.

@CyberShadow
Copy link
Member

Another regression filed against this change:

https://issues.dlang.org/show_bug.cgi?id=14582

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants