Thread overview
What is going on in this code?
Apr 14, 2016
Carlin
Apr 14, 2016
ag0aep6g
Apr 14, 2016
Carlin
Apr 14, 2016
Daniel Kozak
Apr 14, 2016
Daniel Kozak
Apr 14, 2016
Meta
Apr 16, 2016
Jonathan M Davis
Apr 16, 2016
Jonathan M Davis
April 14, 2016
I have encountered a weird error that only seems to occur when building in debug mode. In release everything works fine. I wanted to post here before raising a new issue in case I'm just horribly blind at what is happening here.

---
import std.stdio;
import std.bitmanip;

ubyte[] serialize(uint t)
{
	ubyte[] bytes = nativeToBigEndian(t);
	return bytes;
}

void main()
{
	writeln(serialize(0));
}
---

$ dub --build=debug
Performing "debug" build using dmd for x86.
test ~master: target for configuration "application" is up to date.
To force a rebuild of up-to-date targets, run again with --force.
Running .\test.exe
[12, 254, 24, 0]

$ dub --build=release
Performing "release" build using dmd for x86.
test ~master: target for configuration "application" is up to date.
To force a rebuild of up-to-date targets, run again with --force.
Running .\test.exe
[0, 0, 0, 0]

Why is it returning a different range in debug but not in release?
April 14, 2016
On 14.04.2016 12:14, Carlin wrote:
> import std.stdio;
> import std.bitmanip;
>
> ubyte[] serialize(uint t)
> {
>      ubyte[] bytes = nativeToBigEndian(t);
>      return bytes;
> }
>
> void main()
> {
>      writeln(serialize(0));
> }

Your code is wrong. It's really easy to get wrong, though. Too easy probably.

nativeToBigEndian returns a fixed-size array. That's a value type. By assigning it to a ubyte[], you're slicing the return value. That is, you make a reference to temporary data. The reference becomes invalid as soon the assignment is over, so you're returning a slice to garbage memory.

To make a ubyte[] from the result of nativeToBigEndian, you have to dup it:
----
ubyte[] serialize(uint t)
{
    return nativeToBigEndian(t).dup;
}
----
April 14, 2016
On Thursday, 14 April 2016 at 10:29:43 UTC, ag0aep6g wrote:
> On 14.04.2016 12:14, Carlin wrote:
>> import std.stdio;
>> import std.bitmanip;
>>
>> ubyte[] serialize(uint t)
>> {
>>      ubyte[] bytes = nativeToBigEndian(t);
>>      return bytes;
>> }
>>
>> void main()
>> {
>>      writeln(serialize(0));
>> }
>
> Your code is wrong. It's really easy to get wrong, though. Too easy probably.
>
> nativeToBigEndian returns a fixed-size array. That's a value type. By assigning it to a ubyte[], you're slicing the return value. That is, you make a reference to temporary data. The reference becomes invalid as soon the assignment is over, so you're returning a slice to garbage memory.
>
> To make a ubyte[] from the result of nativeToBigEndian, you have to dup it:
> ----
> ubyte[] serialize(uint t)
> {
>     return nativeToBigEndian(t).dup;
> }
> ----

Thanks, that works perfectly! I couldn't figure it out and I knew I had to be missing something really basic. I still don't understand though why it works in release but not in debug.
April 14, 2016
On Thursday, 14 April 2016 at 13:08:56 UTC, Carlin wrote:
> On Thursday, 14 April 2016 at 10:29:43 UTC, ag0aep6g wrote:
>> [...]
>
> Thanks, that works perfectly! I couldn't figure it out and I knew I had to be missing something really basic. I still don't understand though why it works in release but not in debug.

because with release build dub will enable inlining. So I guess there is a stack frame elimination
April 14, 2016
On 4/14/16 6:29 AM, ag0aep6g wrote:
> On 14.04.2016 12:14, Carlin wrote:
>> import std.stdio;
>> import std.bitmanip;
>>
>> ubyte[] serialize(uint t)
>> {
>>      ubyte[] bytes = nativeToBigEndian(t);
>>      return bytes;
>> }
>>
>> void main()
>> {
>>      writeln(serialize(0));
>> }
>
> Your code is wrong. It's really easy to get wrong, though. Too easy
> probably.
>
> nativeToBigEndian returns a fixed-size array. That's a value type. By
> assigning it to a ubyte[], you're slicing the return value. That is, you
> make a reference to temporary data. The reference becomes invalid as
> soon the assignment is over, so you're returning a slice to garbage memory.
>
> To make a ubyte[] from the result of nativeToBigEndian, you have to dup it:
> ----
> ubyte[] serialize(uint t)
> {
>      return nativeToBigEndian(t).dup;
> }
> ----

That is awful.

I propose we make slicing such a return value implicitly an error. I can't think of a valid reason for allowing it.

I'm not the only one: https://issues.dlang.org/show_bug.cgi?id=12625

-Steve
April 14, 2016
On Thursday, 14 April 2016 at 13:30:42 UTC, Daniel Kozak wrote:
> On Thursday, 14 April 2016 at 13:08:56 UTC, Carlin wrote:
>> On Thursday, 14 April 2016 at 10:29:43 UTC, ag0aep6g wrote:
>>> [...]
>>
>> Thanks, that works perfectly! I couldn't figure it out and I knew I had to be missing something really basic. I still don't understand though why it works in release but not in debug.
>
> because with release build dub will enable inlining. So I guess there is a stack frame elimination

So if you add this:
pragma(inline, false)

before ubyte[] serialize(uint t)

It should not work even with release build type
April 14, 2016
On Thursday, 14 April 2016 at 13:31:25 UTC, Steven Schveighoffer wrote:
> That is awful.
>
> I propose we make slicing such a return value implicitly an error. I can't think of a valid reason for allowing it.
>
> I'm not the only one: https://issues.dlang.org/show_bug.cgi?id=12625
>
> -Steve

Absolutely agreed. Why haven't we done it already?
April 15, 2016
On Thursday, April 14, 2016 09:31:25 Steven Schveighoffer via Digitalmars-d wrote:
> That is awful.
>
> I propose we make slicing such a return value implicitly an error. I can't think of a valid reason for allowing it.
>
> I'm not the only one: https://issues.dlang.org/show_bug.cgi?id=12625

Totally agree, though I wish that we could take it one step further and get rid of the implicit slicing of static arrays. It's an unsafe feature, and once https://issues.dlang.org/show_bug.cgi?id=8838 has been fixed, and slicing a static array is flagged as @system, allowing the implicit slicing makes it really easy to miss that it's happening, and if multiple @system operations are happening in a function, the slicing of the static array could easily be missed by a programmer verifying the safety of the code in order to mark it with @trusted, whereas if the slice were explicit, it would be obvious. Unfortunately, I'm willing to bet that there's no way that Walter would give his okay on making the implicit slicing illegal because of how much code would break as a result.

- Jonathan M Davis

April 15, 2016
On Friday, April 15, 2016 21:16:44 Jonathan M Davis via Digitalmars-d wrote:
> On Thursday, April 14, 2016 09:31:25 Steven Schveighoffer via Digitalmars-d
>
> wrote:
> > That is awful.
> >
> > I propose we make slicing such a return value implicitly an error. I can't think of a valid reason for allowing it.
> >
> > I'm not the only one: https://issues.dlang.org/show_bug.cgi?id=12625
>
> Totally agree, though I wish that we could take it one step further and get rid of the implicit slicing of static arrays. It's an unsafe feature, and once https://issues.dlang.org/show_bug.cgi?id=8838 has been fixed, and slicing a static array is flagged as @system, allowing the implicit slicing makes it really easy to miss that it's happening, and if multiple @system operations are happening in a function, the slicing of the static array could easily be missed by a programmer verifying the safety of the code in order to mark it with @trusted, whereas if the slice were explicit, it would be obvious. Unfortunately, I'm willing to bet that there's no way that Walter would give his okay on making the implicit slicing illegal because of how much code would break as a result.

Well, for whatever it's worth, I created an enhancement request. Maybe we'll get lucky and be able to talk Walter into it:

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

8838 and 12625 still need to be fixed regardless, but if we got rid of the implicit slicing of static arrays, then you'd at least always be able to see what's going on and more easily catch slicing-related bugs.

- Jonathan M Davis