www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - std.digest toHexString

reply Carl Sturtivant <sturtivant gmail.com> writes:
What's going on here?
```
import std.digest.md, std.stdio;

void main() {
     string ans = hex("qwertyuiop");
     writeln(ans);
}

string hex(string arg) {
     string ans = md5Of(arg).toHexString();
     writeln(ans);
     return ans;
}
```
This compiles, and when run writes out corrupt nonsense from the 
writeln in main, while just before that writing out 
6EEA9B7EF19179A06954EDD0F6C05CEB from the function hex.

If I replace md5Of(arg).toHexString() with 
toHexString(md5Of(arg)) it won't compile, claiming that 
"toHexString.d(11): Error: function expected before (), not 
module toHexString of type void". Yet the examples here
http://dlang.org/phobos/std_digest_md.html
use toHexString freely in this way. I thought I was using 
toHexString by UFCS, in the earlier example, but this appears to 
be untrue.

The return from the function hex above somehow corrupts the 
string returned, suggesting deallocation is occurring in some 
fashion. So I replaced md5Of(arg).toHexString() with 
md5Of(arg).toHexString().dup and the problem vanished. Now 
6EEA9B7EF19179A06954EDD0F6C05CEB is printed out twice.
Mar 16 2017
next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 16:13:33 UTC, Carl Sturtivant wrote:

     string ans = md5Of(arg).toHexString();


That is a major D design flaw biting you the same way it has 
bitten so many others.

See the red box in my documentation fork:

http://dpldocs.info/experimental-docs/std.digest.digest.toHexString.2.html

toHexString returns a static array... on the stack. Then the 
stupid language not only implicitly casts it to immutable, it 
also implicitly slices it, giving you a reference to mutable, 
temporary data pretending to be permanent, immutable data.


 If I replace md5Of(arg).toHexString() with 
 toHexString(md5Of(arg))....


That works for me though... maybe it is just a version mismatch 
or something, since toHexString is in a different module than 
md5of.
Mar 16 2017
next sibling parent reply Carl Sturtivant <sturtivant gmail.com> writes:
On Thursday, 16 March 2017 at 16:21:08 UTC, Adam D. Ruppe wrote:

 On Thursday, 16 March 2017 at 16:13:33 UTC, Carl Sturtivant 
 wrote:

     string ans = md5Of(arg).toHexString();


 That is a major D design flaw biting you the same way it has 
 bitten so many others.

 See the red box in my documentation fork:

 http://dpldocs.info/experimental-docs/std.digest.digest.toHexString.2.html

 toHexString returns a static array... on the stack. Then the 
 stupid language not only implicitly casts it to immutable, it 
 also implicitly slices it, giving you a reference to mutable, 
 temporary data pretending to be permanent, immutable data.


Silently <expletive-deleted> cast to immutable without 
copying!??!! This is so wrong.

Yet the documentation says there's a toHexString that returns a 
string.

I don't understand the overload resolution implied at this link. 
How is a toHexString selected in
         string ans = md5Of(arg).toHexString();
?
Mar 16 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 16:47:14 UTC, Carl Sturtivant wrote:

 Silently <expletive-deleted> cast to immutable without 
 copying!??!! This is so wrong.


It is the implicit slicing that I really want removed from the 
language, it serves no real benefit and brings a lot of 
accidental complexity. Casting the static array to immutable is 
actually OK in isolation, because it is a value type and thus a 
unique copy... but once you slice it, that promise is gone.


 Yet the documentation says there's a toHexString that returns a 
 string.


Yes, indeed, it returns a string if it is passed a dynamic array.

Remember though, like I warned on my doc fork, overload 
resolution NEVER looks at the left hand side of the equation, it 
is always done on arguments alone.

The stupid auto return stuff Phobos loves so much obscures it, 
but md5Of returns a ubyte[16].

That calls the first overload, the one that returns char[num*2], 
since the argument most strongly matches the static array one. 
(Interestingly, if it didn't exist, the language would probably 
idiotically slice that ubyte[16] into a ubyte[] and we'd get 
right behavior, for the wrong reason. The implementation of the 
slice one does `new char[]` inside.)

If you explicitly sliced it before the call,

toHexString(md5Of(...)[]); // notice the []

then it would call the one that returns the string and be fine!
Mar 16 2017
next sibling parent reply Carl Sturtivant <sturtivant gmail.com> writes:
On Thursday, 16 March 2017 at 16:59:40 UTC, Adam D. Ruppe wrote:

 Yet the documentation says there's a toHexString that returns 
 a string.


 Yes, indeed, it returns a string if it is passed a dynamic 
 array.


Ah, that's the distinction, should have noticed.


 Remember though, like I warned on my doc fork, overload 
 resolution NEVER looks at the left hand side of the equation, 
 it is always done on arguments alone.

 The stupid auto return stuff Phobos loves so much obscures it, 
 but md5Of returns a ubyte[16].

 That calls the first overload, the one that returns char[num*2]


OK, but if I try to do this,

char[2] u;
string s = u;

the compiler will complain: Error: cannot implicitly convert 
expression (u) of type char[2] to string. So why does it allow 
the template instantiation  of return type char[num*2] in place 
of u above?
Mar 16 2017
parent Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 17:20:10 UTC, Carl Sturtivant wrote:

 OK, but if I try to do this,

 char[2] u;
 string s = u;


Yeah, that's because `u` is not necessarily unique like a 
function return value:

char[2] u;
char[] mutable = u[];
string s = u[];

char before = s[0];

mutable[0] = 'n';

char after = s[0];

assert(before == after); // fails! because mutable[0] changed it.



But, with a function return value, there's no opportunity to 
insert that sneaky `char[] mutable = u[];` alias, so the compiler 
is free to assume that it is the only reference and do the 
immutable thing.

If it returns a value type, the compiler can cast to immutable at 
any time because it knows there is no chance to insert an alias 
at all - the function return is the only way to access it.

If it returns a reference type, the compiler can cast to 
immutable if the function is *strongly* pure, because the purity 
rules also don't give you an opportunity to sneak a mutable 
reference out (pure cannot write to stuff outside the return 
values).

I said *strongly* because the compiler will correctly complain if 
you try to pass a mutable argument back as the return value. It 
realizes whomever passed the argument might still have an alias 
and will not allow the implicit cast:

pure char[] foo(char[] a) { return a; }
char[] lolStillMutable;
string tricky = foo(lolStillMutable); // illegal
Mar 16 2017
prev sibling parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Thu, Mar 16, 2017 at 04:59:40PM +0000, Adam D. Ruppe via Digitalmars-d-learn
wrote:

 On Thursday, 16 March 2017 at 16:47:14 UTC, Carl Sturtivant wrote:

 Silently <expletive-deleted> cast to immutable without copying!??!!
 This is so wrong.


 
 It is the implicit slicing that I really want removed from the
 language, it serves no real benefit and brings a lot of accidental
 complexity. Casting the static array to immutable is actually OK in
 isolation, because it is a value type and thus a unique copy... but
 once you slice it, that promise is gone.


[...]

I'm not convinced casting static array to immutable is OK. Check this
out:

	import std.stdio;

	char[32] func() {
	    char[32] staticArr = "A123456789abcdefB123456789abcdef";
	    return staticArr; // OK, by-value return
	}

	string gunk() {
	    string x = func(); // implicit conversion char[32] -> string
	    writeln(x.ptr);
	    writeln(x);		// prints "A123456789abcdefB123456789abcdef"
	    return x;
	}

	void main() {
	    auto s = gunk();
	    writeln(s.ptr);	// prints same address as in gunk()
	    writeln(s);		// prints corrupted string
	}

Run this code and you'll see that s.ptr has the same address as x.ptr,
and that x.ptr is the address of a local variable. This is blatantly
wrong.

Filed a new issue for this:

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


T

-- 
I am a consultant. My job is to make your job redundant. -- Mr Tom
Mar 16 2017
next sibling parent Carl Sturtivant <sturtivant gmail.com> writes:
On Thursday, 16 March 2017 at 17:20:45 UTC, H. S. Teoh wrote:

 I'm not convinced casting static array to immutable is OK. 
 Check this out:

 	import std.stdio;

 	char[32] func() {
 	    char[32] staticArr = "A123456789abcdefB123456789abcdef";
 	    return staticArr; // OK, by-value return
 	}

 	string gunk() {
 	    string x = func(); // implicit conversion char[32] -> 
 string
 	    writeln(x.ptr);
 	    writeln(x);		// prints "A123456789abcdefB123456789abcdef"
 	    return x;
 	}

 	void main() {
 	    auto s = gunk();
 	    writeln(s.ptr);	// prints same address as in gunk()
 	    writeln(s);		// prints corrupted string
 	}

 Run this code and you'll see that s.ptr has the same address as 
 x.ptr, and that x.ptr is the address of a local variable. This 
 is blatantly wrong.

 Filed a new issue for this:

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


Exactly, if there was a variable of type char[32] on the right 
hand side of
     string x = func();
instead of the call of func, then the compiler would complain. So 
this is a bug.
Mar 16 2017
prev sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 17:20:45 UTC, H. S. Teoh wrote:

 I'm not convinced casting static array to immutable is OK. 
 Check this out:



You aren't casting static array to immutable there, it is being 
implicitly sliced AND cased to immutable.

What I mean is:

char[32] s;
immutable(char)[32] i = s; // that is sane, it copies anyway
immutable(char[32]) i2 = s; // also sane


That's an implicit cast to immutable, but since both sides are 
still static arrays, it is no different than

int a = 0;
immutable b = a; // ok

Value types can be copied in and out of immutable implicitly 
without worry.

It is when they become a reference type that things go wrong. 
And, in isolation, the compiler knows this too:

immutable(char)[] i = s; //  Error: cannot implicitly convert 
expression (s) of type char[32] to string


But when you put it behind a function like you did, two things 
happen:

1) it sees it is a return value by value, and thus unique... so 
it is safe to cast to immutable

char[32] func() {
         char[32] a;
         return a;
}

void main() {
      immutable(char[32]) a = func(); // that's fine! it is copied 
so it is immutable
}

but 2), it is also willing to slice it:

char[] b = func(); // legal under current rules... but insane

and 3) knowing it is a slice of unique data, it allows it to be 
casted just like:

pure char[] foo() { return []; }

string implicit = foo(); // fine - pure slice must be unique due 
to purity rules, and unique mutable can become immutable


Combine all that and we get:

char[32] func() { }

1) char[] implicitlySliced = func(); // stupid, but allowed by 
language rules
2) immutable(char)[] implicitly immutable = impliciltlySliced; // 
ok, because it is unique...
3) crashing in bug city.





Of all those rules, preventing the implicit slice is the easiest 
fix:

string s = func(); // error, CAN implicitly cast from char[32] to 
immutable(char[32]) but can NOT implicitly cast from static to 
dynamic

Being a unique value, casting to immutable is perfectly sane.

immutable(char[32]) s = func(); // makes sense
string s2 = s; // nope, if you really meant it, write `s[]`

But the caller should be aware of when a reference is taken.

string s = func()[]; // I'd allow it, at least the user wrote 
`[]` meaning they realized it was stack data and presumably knows 
what that means about the slice's lifetime
Mar 16 2017
parent reply Kagamin <spam here.lot> writes:
On Thursday, 16 March 2017 at 17:50:45 UTC, Adam D. Ruppe wrote:

 string s = func()[]; // I'd allow it, at least the user wrote 
 `[]` meaning they realized it was stack data and presumably 
 knows what that means about the slice's lifetime


This explicit slice won't work, because a slice of a fixed size 
array results in a fixed size array.

There's no reason to allow this buggy code though. Also 
programmer didn't necessarily mean it. It could be a typographic 
error or a result of refactoring. Even if programmer wanted to 
write invalid code, what for? Yet this particular code doesn't 
need to be allowed to allow writing invalid code: one can still 
cast a number to pointer and do whatever he wants.
Mar 20 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Monday, 20 March 2017 at 15:38:26 UTC, Kagamin wrote:

 This explicit slice won't work, because a slice of a fixed size 
 array results in a fixed size array.


No, it doesn't. int[4] a; typeof(a[] == int[])

You can try yourself in the compiler, it is easy to verify.
Mar 20 2017
parent reply Kagamin <spam here.lot> writes:
On Monday, 20 March 2017 at 15:46:10 UTC, Adam D. Ruppe wrote:

 On Monday, 20 March 2017 at 15:38:26 UTC, Kagamin wrote:

 This explicit slice won't work, because a slice of a fixed 
 size array results in a fixed size array.


 No, it doesn't. int[4] a; typeof(a[] == int[])

 You can try yourself in the compiler, it is easy to verify.


https://dpaste.dzfl.pl/eafa86c5426d
Mar 20 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Monday, 20 March 2017 at 16:04:10 UTC, Kagamin wrote:

 https://dpaste.dzfl.pl/eafa86c5426d


Unbelievable, we're both right, sort of.

So it is true that typeof(static[]) == dynamic.

But the language also allows implicit conversion in the other 
direction.... WTF. If you put a variable in between, it will do a 
runtime array copy with assert(sizes match), and if the compiler 
can keep track of where it came from, it will implicitly just 
make it happen.

So we explicitly sliced... then the compiler implicitly undid it 
again for overload selection since it knew the size.

This might be the most messed up part of the D language to me now.
Mar 20 2017
parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 03/20/2017 09:31 AM, Adam D. Ruppe wrote:

 On Monday, 20 March 2017 at 16:04:10 UTC, Kagamin wrote:

 https://dpaste.dzfl.pl/eafa86c5426d


 Unbelievable, we're both right, sort of.

 So it is true that typeof(static[]) == dynamic.

 But the language also allows implicit conversion in the other
 direction.... WTF. If you put a variable in between, it will do a
 runtime array copy with assert(sizes match), and if the compiler can
 keep track of where it came from, it will implicitly just make it happen.

 So we explicitly sliced... then the compiler implicitly undid it again
 for overload selection since it knew the size.

 This might be the most messed up part of the D language to me now.


Agreed. Surprisingly, there are quite a number of issues that request 
exactly that, mostly thanks to our old friend bearophile. The following 
may be the reason for this WAT:

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

Ali
Mar 20 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Monday, 20 March 2017 at 21:04:30 UTC, Ali Çehreli wrote:

 Agreed. Surprisingly, there are quite a number of issues that 
 request exactly that, mostly thanks to our old friend 
 bearophile. The following may be the reason for this WAT:


This is another case where I can kinda get it in isolation, but 
it is weird coming together with everything else. The propagation 
of size through the expression... but it is a bit weird that 
explicitly slicing it *still* prefers the static size version.... 
but ONLY when the compiler can prove the size. Break it into two 
expressions, and you get the other overload again.

The rules make sense alone, but together, they are just bizarre.
Mar 20 2017
parent Jonathan M Davis via Digitalmars-d-learn writes:
On Monday, March 20, 2017 22:47:24 Adam D. Ruppe via Digitalmars-d-learn 
wrote:

 The rules make sense alone, but together, they are just bizarre.


That's frequently where the language design pitfalls lie. Well-meaning
features that seem perfectly reasonable on their own (possibly even
unreasonable to not have) interact badly in practice. Sometimes, they're
caught, and sometimes they're not. A number of the annoying restrictions in
D are there precisely to try and avoid those problems like that that turned
up in C++. But we've managed to add new ones. Hopefully, this one can still
be fixed.

- Jonathan M Davis
Mar 20 2017
prev sibling next sibling parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Thu, Mar 16, 2017 at 04:21:08PM +0000, Adam D. Ruppe via Digitalmars-d-learn
wrote:

 On Thursday, 16 March 2017 at 16:13:33 UTC, Carl Sturtivant wrote:

     string ans = md5Of(arg).toHexString();


 
 That is a major D design flaw biting you the same way it has bitten so
 many others.
 
 See the red box in my documentation fork:
 
 http://dpldocs.info/experimental-docs/std.digest.digest.toHexString.2.html
 
 toHexString returns a static array... on the stack. Then the stupid
 language not only implicitly casts it to immutable, it also implicitly
 slices it, giving you a reference to mutable, temporary data
 pretending to be permanent, immutable data.


WAT?!  Why does the language allow implicitly casting from static array
to string?!  How is that even remotely correct?  Is there a bug filed
for this?


T

-- 
To provoke is to call someone stupid; to argue is to call each other stupid.
Mar 16 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 16:46:43 UTC, H. S. Teoh wrote:

 WAT?!  Why does the language allow implicitly casting from 
 static array to string?!  How is that even remotely correct?  
 Is there a bug filed for this?


Yeah, somewhere, bugzilla search sucks but I know it is in there 
somewhere. We've talked about it at some length, including JMD 
and I recently on github relating to the Phobos template 
constraint mess: 
https://github.com/dlang/phobos/pull/5259#issuecomment-285445535

He said Walter thinks one of those memory safety DIPs will moot 
it, but I completely disagree. Even if this code threw an error 
(which it ABSOLUTELY SHOULD for several reasons), the implicit 
slice still leads to convoluted Phobos constraints to deal with 
it sanely! (Well, IMO the sane option is to just disallow it in 
the constraint, but ohes noes teh broken c0dez)

In isolation, implicit slicing can make sense.... but not with 
templates. In isolation, implicit immutable can make sense... but 
not with implicit slicing.

We aren't getting rid of templates. They are useful. But implicit 
slicing? Please, just write `[]` if you want that, easy fix, and 
then we'd rescue the immutable cast too.
Mar 16 2017
next sibling parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Thu, Mar 16, 2017 at 05:06:39PM +0000, Adam D. Ruppe via Digitalmars-d-learn
wrote:
[...]

 In isolation, implicit slicing can make sense.... but not with
 templates. In isolation, implicit immutable can make sense... but not
 with implicit slicing.


[...]

Is implicit slicing the culprit in the issue I just filed? (Bug 17261)

In retrospect, perhaps implicit casting to immutable is OK if we don't
allow implicit slicing:

	char[32] func() { char[32] s; return s; }
	string gunk() {
		string x = func(); // error, if implicit slicing is not allowed
		return x;	// if allowed, this causes escaping ref to stack data
	}
	immutable char[32] hunk() {
		immutable char[32] x = func(); // should be OK: no implicit slicing
		return x; // OK: return by-value, no escaping refs
	}
	string junk() {
		immutable char[32] x = func(); // should be OK: no implicit slicing
		return x; // NG: implicit slicing causes escaping ref
			// However, compiler is smart enough to catch
			// it, so it produces a compile error
	}

Seems like the real cause of bug 17261 is implicit slicing.


T

-- 
Debugging is twice as hard as writing the code in the first place. Therefore,
if you write the code as cleverly as possible, you are, by definition, not
smart enough to debug it. -- Brian W. Kernighan
Mar 16 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 17:40:51 UTC, H. S. Teoh wrote:

 Seems like the real cause of bug 17261 is implicit slicing.


Yes, sorry, it took me 10 mins to type it up (I like to double 
check the current behavior before posting) but I think we both 
see it the same way now.
Mar 16 2017
parent reply Petar Kirov [ZombineDev] <petar.p.kirov gmail.com> writes:
On Thursday, 16 March 2017 at 17:51:41 UTC, Adam D. Ruppe wrote:

 On Thursday, 16 March 2017 at 17:40:51 UTC, H. S. Teoh wrote:

 Seems like the real cause of bug 17261 is implicit slicing.


 Yes, sorry, it took me 10 mins to type it up (I like to double 
 check the current behavior before posting) but I think we both 
 see it the same way now.


Guys, seriously this bug has been fixed for quite some time:
https://issues.dlang.org/show_bug.cgi?id=8838
https://issues.dlang.org/show_bug.cgi?id=12625
https://github.com/dlang/dmd/pull/5972

Why don't you use -dip1000???

/home/zombinedev/dlang/dmd-2.073.2/linux/bin64/../../src/phobo
/std/stdio.d(3439): Error: template std.stdio.File.LockingTextWriter.put cannot
deduce function from argument types !()(string), candidates are:
/home/zombinedev/dlang/dmd-2.073.2/linux/bin64/../../src/phobo
/std/stdio.d(2643):        std.stdio.File.LockingTextWriter.put(A)(A writeme)
if (is(ElementType!A : const(dchar)) && isInputRange!A && !isInfinite!A)
/home/zombinedev/dlang/dmd-2.073.2/linux/bin64/../../src/phobo
/std/stdio.d(2672):        std.stdio.File.LockingTextWriter.put(C)(C c) if
(is(C : const(dchar)))
scope_test.d(5): Error: template instance 
std.stdio.writeln!string error instantiating
scope_test.d(11): Error: scope variable ans may not be returned

The only issue is that not enough people (except Walter & Andrei) 
are interested in pushing -dip1000 as the default. Druntime 
already compiles with -dip1000, so Phobos is the next frontier.
Mar 16 2017
next sibling parent Petar Kirov [ZombineDev] <petar.p.kirov gmail.com> writes:
On Thursday, 16 March 2017 at 18:07:09 UTC, Petar Kirov 
[ZombineDev] wrote:

 On Thursday, 16 March 2017 at 17:51:41 UTC, Adam D. Ruppe wrote:

 On Thursday, 16 March 2017 at 17:40:51 UTC, H. S. Teoh wrote:

 Seems like the real cause of bug 17261 is implicit slicing.


 Yes, sorry, it took me 10 mins to type it up (I like to double 
 check the current behavior before posting) but I think we both 
 see it the same way now.


 Guys, seriously this bug has been fixed for quite some time:
 https://issues.dlang.org/show_bug.cgi?id=8838
 https://issues.dlang.org/show_bug.cgi?id=12625
 https://github.com/dlang/dmd/pull/5972

 Why don't you use -dip1000???

 /home/zombinedev/dlang/dmd-2.073.2/linux/bin64/../../src/phobo
/std/stdio.d(3439): Error: template std.stdio.File.LockingTextWriter.put cannot
deduce function from argument types !()(string), candidates are:
 /home/zombinedev/dlang/dmd-2.073.2/linux/bin64/../../src/phobo
/std/stdio.d(2643):        std.stdio.File.LockingTextWriter.put(A)(A writeme)
if (is(ElementType!A : const(dchar)) && isInputRange!A && !isInfinite!A)
 /home/zombinedev/dlang/dmd-2.073.2/linux/bin64/../../src/phobo
/std/stdio.d(2672):        std.stdio.File.LockingTextWriter.put(C)(C c) if
(is(C : const(dchar)))
 scope_test.d(5): Error: template instance 
 std.stdio.writeln!string error instantiating
 scope_test.d(11): Error: scope variable ans may not be returned

 The only issue is that not enough people (except Walter & 
 Andrei) are interested in pushing -dip1000 as the default. 
 Druntime already compiles with -dip1000, so Phobos is the next 
 frontier.


See also 
https://github.com/dlang/dmd/pulls?utf8=%E2%9C%93&q=is%3Apr%20is%3Aclosed%20scope%20author%3AWalterBright%20
Mar 16 2017
prev sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 18:07:09 UTC, Petar Kirov 
[ZombineDev] wrote:

 Why don't you use -dip1000???


Because it isn't the default. But even if it was, people would 
ask "why is this giving me an error?" and the same explanation 
would need to be given. Certainly, a compile error is better than 
runtime corruption, but the underlying issue ought to be fixed 
anyway.

Phobos could have been written to avoid this problem too, there's 
a few solutions that work, but right now, using the standard 
library in a way people expect to work will be silently 
disastrous.

It is a good study of 3 language features, two of which I think 
are somewhat brilliant... and the third which is a big pain in a 
lot of ways for no real benefit. So I want to kill that design 
flaw.
Mar 16 2017
next sibling parent reply Carl Sturtivant <sturtivant gmail.com> writes:
On Thursday, 16 March 2017 at 18:51:45 UTC, Adam D. Ruppe wrote:

 Phobos could have been written to avoid this problem too, 
 there's a few solutions that work, but right now, using the 
 standard library in a way people expect to work will be 
 silently disastrous.


Yes, and as an outsider I find this, well, disconcerting. No 
complaint from the compiler about assignment to string of the 
result of a library function call should have produced a string 
with the obvious semantics.

Having read this thread I have formed a conclusion.

Implicitly slicing rvalue arrays is too much like implicitly 
taking the address of an rvalue.

There's an explicit postfix operator [] to do that, and if there 
was no implicit slicing, I'd at least know where slicing is 
occurring and I wouldn't use the slice of a temporary beyond its 
lifetime.

Now a function with a slice parameter could not be called with an 
rvalue array parameter without putting an explicit slice operator 
in at the point of call.

But writing a function like that is just a way to regard rvalue 
arrays of different sizes based upon the same type as being the 
same type, when they are not. They are distinct types. And so a 
template could take care of that minor syntactic problem if so 
desired, with one instantiation for each rvalue array type (i.e. 
size), with a ref parameter to avoid copying.

I see every reason to remove implicit slicing of rvalue arrays.

Trying to keep it available sometimes is a complex endeavor, and 
the rules will be lengthy, and consequently have more 
complications to explain to people joining use of D, and for 
what? There's almost nothing to gain. This would be a mistake. D 
is already very large.

If I didn't know what my general confidence level in D was for 
other reasons, this incident could well have driven me away. The 
standard library compiled completely unexpectedly insane and 
unsafe semantics when I just called a simple-looking function. 
This sort of thing is undoubtedly bringing D into disrepute with 
some people here and there, people just trying it out to solve a 
problem.
Mar 16 2017
next sibling parent Jonathan M Davis via Digitalmars-d-learn writes:
On Thursday, March 16, 2017 20:42:21 Carl Sturtivant via Digitalmars-d-learn 
wrote:

 Implicitly slicing rvalue arrays is too much like implicitly
 taking the address of an rvalue.


Well, that's just it. That's _exactly_ what's happening. It's just that it
ends up in a struct with a length member along with it. You have something
like

struct Array(T)
{
    size_t length;
    T* ptr;
}

instead of

T* ptr;

In both cases, ptr refers to the same address, and each case is exactly as
 safe as the other - as in, not at all.

Unfortunately, for some reason, slicing static arrays has historically been
considered  safe, whereas taking an address of a local variable is
considered  systeme even though they're doing _exactly_ the same thing
except that the slicing ends up with a struct with length instead of just
with a pointer. Fortunately, the  safety improvements that Walter has been
working on should finally fix that.


 I see every reason to remove implicit slicing of rvalue arrays.


Honestly, I think that it was a big mistake to have implicit slicing of
static arrays in the language at all. Unfortunately, last time I tried to
convince Walter of that, he seemed to think that the  safety improvements to
the compiler were going to fix the problem, and they will help, but I'm of
the opinion that slicing of static arrays should always be explicit. It's
too easy to miss what's going on otherwise, even if it isn't an  safety
problem.

In any case, the  safety fixes that are underway should eventually at least
flag this as  system if not result in it being outright illegal. And I think
that there's a good case to make any variant of this issue be illegal if
it's guaranteed that you're going to end up with a pointer to invalid
memory.

- Jonathan M Davis
Mar 16 2017
prev sibling parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Thu, Mar 16, 2017 at 02:36:15PM -0700, Jonathan M Davis via
Digitalmars-d-learn wrote:
[...]

 Honestly, I think that it was a big mistake to have implicit slicing
 of static arrays in the language at all.


That makes at least 3 of us -- Adam, you, me. I'm sure there are more.



 Unfortunately, last time I tried to convince Walter of that, he seemed
 to think that the  safety improvements to the compiler were going to
 fix the problem, and they will help, but I'm of the opinion that
 slicing of static arrays should always be explicit. It's too easy to
 miss what's going on otherwise, even if it isn't an  safety problem.


Yeah, it's one of those things that seemed like a good idea on the
surface, like autodecoding, but the deeper you go, the more troubles you
discover.  Unlike autodecoding, though, any code breakage caused by
killing autoslicing of static arrays will, I speculate, only affect
already-buggy code that ought to be fixed anyway.  And AFAIK, nobody has
argued *for* implicit slicing of static arrays.



 In any case, the  safety fixes that are underway should eventually at
 least flag this as  system if not result in it being outright illegal.
 And I think that there's a good case to make any variant of this issue
 be illegal if it's guaranteed that you're going to end up with a
 pointer to invalid memory.


[...]

The wrinkle is that the way dmd currently implements this, AFAICT, is to
leave returned static array rvalues on the stack until the end of the
function, even if it's inside a nested sub-scope that is followed by
other nested sub-scopes that may go even deeper. It's not wrong to do
this -- there's no requirement per se that stack variables have to be
freed immediately upon leaving a nested scope so that the stack space
can be reused by subsequent local variables. But it does obscure this
particular issue by hiding the problem of an escaping reference to an
rvalue that has long gone out of scope.

Not even -dip1000 catches this case right now, which I think is a bug,
because the way I understand DIP 1000 is that this should be illegal.


T

-- 
It won't be covered in the book. The source code has to be useful for
something, after all. -- Larry Wall
Mar 16 2017
parent NotSpooky <asistentedeprincipios2 gmail.com> writes:
On Thursday, 16 March 2017 at 22:06:24 UTC, H. S. Teoh wrote:

 On Thu, Mar 16, 2017 at 02:36:15PM -0700, Jonathan M Davis via 
 Digitalmars-d-learn wrote: [...]

 Honestly, I think that it was a big mistake to have implicit 
 slicing of static arrays in the language at all.


 That makes at least 3 of us -- Adam, you, me. I'm sure there 
 are more.


Make it 4.
Mar 18 2017
prev sibling parent "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Thu, Mar 16, 2017 at 06:51:45PM +0000, Adam D. Ruppe via Digitalmars-d-learn
wrote:

 On Thursday, 16 March 2017 at 18:07:09 UTC, Petar Kirov [ZombineDev] wrote:

 Why don't you use -dip1000???


 
 Because it isn't the default. But even if it was, people would ask
 "why is this giving me an error?" and the same explanation would need
 to be given.  Certainly, a compile error is better than runtime
 corruption, but the underlying issue ought to be fixed anyway.


Not to mention that even with -dip1000, the fundamental problem still
happens:

	int[16] func();
	int[] x = func();	// allowed even with -dip1000

	x[0] = 123; // this writes to the now out-of-scope rvalue returned by func()


T

-- 
Doubtless it is a good thing to have an open mind, but a truly open mind should
be open at both ends, like the food-pipe, with the capacity for excretion as
well as absorption. -- Northrop Frye
Mar 16 2017
prev sibling parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Thu, Mar 16, 2017 at 10:40:51AM -0700, H. S. Teoh via Digitalmars-d-learn
wrote:
[...]

 Is implicit slicing the culprit in the issue I just filed? (Bug 17261)
 
 In retrospect, perhaps implicit casting to immutable is OK if we don't
 allow implicit slicing:
 
 	char[32] func() { char[32] s; return s; }
 	string gunk() {
 		string x = func(); // error, if implicit slicing is not allowed


Actually, the bug still exists even if you explicitly slice it:

	string x = func()[]; // still compiles, but shouldn't

For some reason, slicing a static array return value is somehow OK,
while slicing a local variable is rejected.  Seems like the compiler is
missing escaping ref checks for return values?


T

-- 
People tell me I'm stubborn, but I refuse to accept it!
Mar 16 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 17:47:34 UTC, H. S. Teoh wrote:

 Actually, the bug still exists even if you explicitly slice it:

 	string x = func()[]; // still compiles, but shouldn't


I don't call that a bug, once it is explicitly done it means the 
programmer realized something is up and decided they are OK with 
it. Perhaps you want to pass it to a function that you know isn't 
going to hold the reference beyond the calling scope.


 For some reason, slicing a static array return value is somehow 
 OK, while slicing a local variable is rejected.  Seems like the 
 compiler is missing escaping ref checks for return values?


It's the uniqueness thing, see my last email (I probably was 
typing it at the same time you were typing this...)

This isn't an escape per se, `string x` is still a local variable.

immutable(char)[32] buffer;
string s = buffer[0 .. 16]; // sane and really useful 
optimization... just be careful not to escape it

Walter wants to expand the escape check so it automatically 
issues an error if you aren't careful enough, but the status quo 
is still usable - such code is not necessarily wrong, banning 
entirely it is a step backward, and programmers coming up the C 
tradition are used to watching lifetimes like that.
Mar 16 2017
parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Thu, Mar 16, 2017 at 06:09:52PM +0000, Adam D. Ruppe via Digitalmars-d-learn
wrote:

 On Thursday, 16 March 2017 at 17:47:34 UTC, H. S. Teoh wrote:

 Actually, the bug still exists even if you explicitly slice it:
 
 	string x = func()[]; // still compiles, but shouldn't


 
 I don't call that a bug, once it is explicitly done it means the
 programmer realized something is up and decided they are OK with it.
 Perhaps you want to pass it to a function that you know isn't going to
 hold the reference beyond the calling scope.
 

 For some reason, slicing a static array return value is somehow OK,
 while slicing a local variable is rejected.  Seems like the compiler
 is missing escaping ref checks for return values?


 
 It's the uniqueness thing, see my last email (I probably was typing it
 at the same time you were typing this...)
 
 This isn't an escape per se, `string x` is still a local variable.
 
 immutable(char)[32] buffer;
 string s = buffer[0 .. 16]; // sane and really useful optimization... just
 be careful not to escape it
 
 Walter wants to expand the escape check so it automatically issues an
 error if you aren't careful enough, but the status quo is still usable
 - such code is not necessarily wrong, banning entirely it is a step
 backward, and programmers coming up the C tradition are used to
 watching lifetimes like that.


Ah, you're right.  And as somebody indicated in the bug comments,
compiling with -dip1000 correctly rejects the `return s;` line as trying
to return a reference to something that's going out of scope.

But in that case, wouldn't that mean implicit slicing isn't to blame
here? (Not that I'm arguing for implicit slicing -- I think it needs to
go, too, having been bitten by it before -- but this particular case
wouldn't constitute as evidence against it.)


T

-- 
Meat: euphemism for dead animal. -- Flora
Mar 16 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 18:10:43 UTC, H. S. Teoh wrote:

 But in that case, wouldn't that mean implicit slicing isn't to 
 blame here? (Not that I'm arguing for implicit slicing -- I 
 think it needs to go, too, having been bitten by it before -- 
 but this particular case wouldn't constitute as evidence 
 against it.)


The implicit slicing is still bug prone, just dip1000 catches it 
before it snowballs. That's a good thing, dip1000 puts an 
additional wall between the termites and the wood, but the 
termites do remain.
Mar 16 2017
parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Thu, Mar 16, 2017 at 06:41:40PM +0000, Adam D. Ruppe via Digitalmars-d-learn
wrote:

 On Thursday, 16 March 2017 at 18:10:43 UTC, H. S. Teoh wrote:

 But in that case, wouldn't that mean implicit slicing isn't to blame
 here? (Not that I'm arguing for implicit slicing -- I think it needs
 to go, too, having been bitten by it before -- but this particular
 case wouldn't constitute as evidence against it.)


 
 The implicit slicing is still bug prone, just dip1000 catches it
 before it snowballs. That's a good thing, dip1000 puts an additional
 wall between the termites and the wood, but the termites do remain.


Actually, https://issues.dlang.org/show_bug.cgi?id=12625 shows that
implicit slicing is still a problem:

	char[16] func() { ... }
	void gunk() {
		string s = func(); // implicit slice

		... // do some stuff that uses the stack

		// since func's return value is a temporary, it has gone
		// out of scope here, and there is no guarantee it
		// hasn't already been overwritten by this point, right
		// inside gunk's body!  So s may already have corrupted
		// data.
	}

The problem is that func's return value is an rvalue, so slicing it
means we're already escaping a reference to data that's going out of
scope (by the end of the expression). It's no different from trying to
take the address of an rvalue.

Seems this isn't caught by -dip1000, though from what I understand of
DIP 1000, this *should* have been rejected.


T

-- 
People who are more than casually interested in computers should have at least
some idea of what the underlying hardware is like. Otherwise the programs they
write will be pretty weird. -- D. Knuth
Mar 16 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 19:00:08 UTC, H. S. Teoh wrote:

 Actually, https://issues.dlang.org/show_bug.cgi?id=12625 shows 
 that implicit slicing is still a problem:


Oh yikes, I figured it would still be copied onto the local stack 
even though it is an rvalue.

But you know, the fact that so many of us are still surprised by 
just what is going on here tells me it is iffy anyway. 
Complicated rules can be fine when they bring compelling 
benefits, but this has a lot of cost and no serious benefit - 
just slice explicitly when you want to!


 Seems this isn't caught by -dip1000, though from what I 
 understand of DIP 1000, this *should* have been rejected.


yea perhaps there is a bug there too.
Mar 16 2017
parent "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Thu, Mar 16, 2017 at 07:17:41PM +0000, Adam D. Ruppe via Digitalmars-d-learn
wrote:

 On Thursday, 16 March 2017 at 19:00:08 UTC, H. S. Teoh wrote:

 Actually, https://issues.dlang.org/show_bug.cgi?id=12625 shows that
 implicit slicing is still a problem:


 
 Oh yikes, I figured it would still be copied onto the local stack even
 though it is an rvalue.


[...]

Well, yes, it would be copied to the local stack once func() returns,
but the problem is, for how long.  It may be that the current
implementation in dmd keeps that stack location allocated until the
function returns, but it would be equally valid to reuse that stack
location after the expression is over, because by definition, rvalues in
the expression would have gone out of scope.


T

-- 
Computers are like a jungle: they have monitor lizards, rams, mice, c-moss,
binary trees... and bugs.
Mar 16 2017
prev sibling parent reply Carl Sturtivant <sturtivant gmail.com> writes:
On Thursday, 16 March 2017 at 16:21:08 UTC, Adam D. Ruppe wrote:

 If I replace md5Of(arg).toHexString() with 
 toHexString(md5Of(arg))....


 That works for me though... maybe it is just a version mismatch 
 or something, since toHexString is in a different module than 
 md5of.


I have this problem on Linux and Windows, at 64 bits on the 
former, and 32 on the latter, with simple clean installations of 
dmd 2.073.2 on Linux, 2.073.3 on Windows. Same error message.
"toHexString.d(11): Error: function expected before (), not 
module toHexString of type void"
Module???
Mar 16 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 16:56:11 UTC, Carl Sturtivant wrote:

 "toHexString.d(11): Error: function expected before (), not 
 module toHexString of type void"
 Module???


huh idk.

use my search engine btw!  http://dpldocs.info/toHexString

it is in `std.digest.digest` but that should be publicly imported.
Mar 16 2017
parent reply Carl Sturtivant <sturtivant gmail.com> writes:
On Thursday, 16 March 2017 at 17:01:56 UTC, Adam D. Ruppe wrote:

 On Thursday, 16 March 2017 at 16:56:11 UTC, Carl Sturtivant 
 wrote:

 "toHexString.d(11): Error: function expected before (), not 
 module toHexString of type void"
 Module???


 huh idk.

 use my search engine btw!  http://dpldocs.info/toHexString

 it is in `std.digest.digest` but that should be publicly 
 imported.


I did that, and it made no difference. :(
Mar 16 2017
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 16 March 2017 at 17:12:08 UTC, Carl Sturtivant wrote:

 I did that, and it made no difference. :(


wait a minute i just realized:

toHexString.d(11) in the error message

You named the file toHexString which means the *module* will be 
named toHexString by default...

So using the name like that the compiler will think you are 
referring to the module (of type void) instead of the function.

I'd say rename that module, but you can also use a 
fully-qualified name to disambiguate like 
`std.digest.digest.toHexString(md5Of(...))`

renaming the module is prolly easiest. either rename the file or 
throw a `module tohexstring;` up top. This btw is one of the 
reasons why the D style guide generally recommends all-lowercase 
module names, to make such conflicts less likely.
Mar 16 2017
parent Carl Sturtivant <sturtivant gmail.com> writes:
On Thursday, 16 March 2017 at 17:18:30 UTC, Adam D. Ruppe wrote:

 On Thursday, 16 March 2017 at 17:12:08 UTC, Carl Sturtivant 
 wrote:

 I did that, and it made no difference. :(


 wait a minute i just realized:

 toHexString.d(11) in the error message

 You named the file toHexString which means the *module* will be 
 named toHexString by default...


OK, right!
Mar 16 2017
prev sibling parent reply Vladimir Panteleev <thecybershadow.lists gmail.com> writes:
On Thursday, 16 March 2017 at 16:13:33 UTC, Carl Sturtivant wrote:

 What's going on here?


Looks like this bug:

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

Has it not been fixed?
Mar 19 2017
parent Kagamin <spam here.lot> writes:
On Monday, 20 March 2017 at 04:03:20 UTC, Vladimir Panteleev 
wrote:

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

 Has it not been fixed?


That's specific to the return statement. Like you can assign an 
address of a local variable, but you can't return it.
Mar 20 2017