www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Implicit static->dynamic arr and modifying

reply Nick Sabalausky <SeeWebsiteToContactMe semitwist.com> writes:
Is this kinds stuff a sane thing to do, or does it just work by accident?:

void modify(ubyte[] dynamicArr)
{
     dynamicArr[$-1] = 5;
}

void main()
{
     ubyte[4] staticArr = [1,1,1,1];
     modify(staticArr);
     assert(staticArr == [1,1,1,5]);
}
May 05 2014
parent reply Nick Sabalausky <SeeWebsiteToContactMe semitwist.com> writes:
On 5/5/2014 10:11 PM, Nick Sabalausky wrote:

 Is this kinds stuff a sane thing to do, or does it just work by accident?:

 void modify(ubyte[] dynamicArr)
 {
      dynamicArr[$-1] = 5;
 }

 void main()
 {
      ubyte[4] staticArr = [1,1,1,1];
      modify(staticArr);
      assert(staticArr == [1,1,1,5]);
 }


Duh, it's just using a normal slice of the static array...

// Roughly:
dynamicArr.ptr = &staticArr;
dynamicArr.length = typeof(staticArr).sizeof;

So all is well, and deliberately so. Pardon the noise.
May 05 2014
next sibling parent Jonathan M Davis via Digitalmars-d-learn writes:
On Mon, 05 May 2014 22:16:58 -0400
Nick Sabalausky via Digitalmars-d-learn
<digitalmars-d-learn puremagic.com> wrote:


 On 5/5/2014 10:11 PM, Nick Sabalausky wrote:

 Is this kinds stuff a sane thing to do, or does it just work by
 accident?:

 void modify(ubyte[] dynamicArr)
 {
      dynamicArr[$-1] = 5;
 }

 void main()
 {
      ubyte[4] staticArr = [1,1,1,1];
      modify(staticArr);
      assert(staticArr == [1,1,1,5]);
 }


 Duh, it's just using a normal slice of the static array...

 // Roughly:
 dynamicArr.ptr = &staticArr;
 dynamicArr.length = typeof(staticArr).sizeof;

 So all is well, and deliberately so. Pardon the noise.


It's definitely deliberate, though I think that it's a flaw in the language's
design. IMHO, static arrays should never be automatically sliced, but
unfortunately, changing that would break too much code at this point. The
biggest problem is the fact that it's inherently unsafe, though unfortunately,
the compiler currently considers it  safe:

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

- Jonathan M Davis
May 06 2014
prev sibling next sibling parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Tue, May 06, 2014 at 01:06:14AM -0700, Jonathan M Davis via
Digitalmars-d-learn wrote:

 On Mon, 05 May 2014 22:16:58 -0400
 Nick Sabalausky via Digitalmars-d-learn
 <digitalmars-d-learn puremagic.com> wrote:
 

 On 5/5/2014 10:11 PM, Nick Sabalausky wrote:

 Is this kinds stuff a sane thing to do, or does it just work by
 accident?:

 void modify(ubyte[] dynamicArr)
 {
      dynamicArr[$-1] = 5;
 }

 void main()
 {
      ubyte[4] staticArr = [1,1,1,1];
      modify(staticArr);
      assert(staticArr == [1,1,1,5]);
 }


 Duh, it's just using a normal slice of the static array...

 // Roughly:
 dynamicArr.ptr = &staticArr;
 dynamicArr.length = typeof(staticArr).sizeof;

 So all is well, and deliberately so. Pardon the noise.


 
 It's definitely deliberate, though I think that it's a flaw in the
 language's design. IMHO, static arrays should never be automatically
 sliced, but unfortunately, changing that would break too much code at
 this point. The biggest problem is the fact that it's inherently
 unsafe, though unfortunately, the compiler currently considers it
  safe:
 
 https://issues.dlang.org/show_bug.cgi?id=8838


[...]

A particularly pernicious instance of this hole is the following:

	class C {
		int[] data;
		this(int[] args...)  safe {
			data = args;
		}
	}

	C f()  safe {
		return new C(1,2,3);
	}

	void main() {
		import std.stdio;
		writeln(f().data); // on my system, writes garbage
	}

Exercise for the reader: spot the bug.


T

-- 
English has the lovely word "defenestrate", meaning "to execute by
throwing someone out a window", or more recently "to remove Windows from
a computer and replace it with something useful". :-) -- John Cowan
May 06 2014
parent "bearophile" <bearophileHUGS lycos.com> writes:
H. S. Teoh:


 Exercise for the reader: spot the bug.


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

Bye,
bearophile
May 06 2014
prev sibling parent reply "Rene Zwanenburg" <renezwanenburg gmail.com> writes:
On Tuesday, 6 May 2014 at 02:17:06 UTC, Nick Sabalausky wrote:

 So all is well, and deliberately so. Pardon the noise.


IMO it's not. I once had a particularly nasty bug because of this:

struct S
{
	 safe:
	string str;
	
	this(string data)
	{
		import std.digest.md;
		str = md5Of(data).toHexString(); // Oops...
	}
}
May 06 2014
parent reply Nick Sabalausky <SeeWebsiteToContactMe semitwist.com> writes:
On 5/6/2014 6:46 PM, Rene Zwanenburg wrote:

 On Tuesday, 6 May 2014 at 02:17:06 UTC, Nick Sabalausky wrote:

 So all is well, and deliberately so. Pardon the noise.


 IMO it's not. I once had a particularly nasty bug because of this:

 struct S
 {
       safe:
      string str;

      this(string data)
      {
          import std.digest.md;
          str = md5Of(data).toHexString(); // Oops...
      }
 }


That must be a terribly subtle one, I'm not seeing the problem at all.

I get that md5Of returns a static array, and then a slice of it gets 
passed to toHexString, but AIUI toHexString finishes (and returns a 
newly allocated string) before the temporary static array leaves scope.
May 07 2014
parent reply "Rene Zwanenburg" <renezwanenburg gmail.com> writes:
On Wednesday, 7 May 2014 at 15:41:19 UTC, Nick Sabalausky wrote:

 On 5/6/2014 6:46 PM, Rene Zwanenburg wrote:

 On Tuesday, 6 May 2014 at 02:17:06 UTC, Nick Sabalausky wrote:

 So all is well, and deliberately so. Pardon the noise.


 IMO it's not. I once had a particularly nasty bug because of 
 this:

 struct S
 {
      safe:
     string str;

     this(string data)
     {
         import std.digest.md;
         str = md5Of(data).toHexString(); // Oops...
     }
 }


 That must be a terribly subtle one, I'm not seeing the problem 
 at all.

 I get that md5Of returns a static array, and then a slice of it 
 gets passed to toHexString, but AIUI toHexString finishes (and 
 returns a newly allocated string) before the temporary static 
 array leaves scope.


toHexString has an overload that takes a static array and can 
therefore return a static array (the length is known to be twice 
the input length). In essence it's the same bug as directly 
storing the result of md5Of, but this was the exact line that was 
causing me grief. Indeed, it looks innocent enough..

So, toHexString returns a static array, which can be implicitly 
assigned to a member slice. In  safe code. I was horrified ;). 
Imo it's one of the most serious violations of D's safe by 
default principle.
May 07 2014
parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Wed, May 07, 2014 at 06:31:15PM +0000, Rene Zwanenburg via
Digitalmars-d-learn wrote:

 On Wednesday, 7 May 2014 at 15:41:19 UTC, Nick Sabalausky wrote:

On 5/6/2014 6:46 PM, Rene Zwanenburg wrote:




[...]

struct S
{
     safe:
    string str;

    this(string data)
    {
        import std.digest.md;
        str = md5Of(data).toHexString(); // Oops...
    }
}


That must be a terribly subtle one, I'm not seeing the problem at
all.

I get that md5Of returns a static array, and then a slice of it gets
passed to toHexString, but AIUI toHexString finishes (and returns a
newly allocated string) before the temporary static array leaves
scope.


 
 toHexString has an overload that takes a static array and can
 therefore return a static array (the length is known to be twice the
 input length). In essence it's the same bug as directly storing the
 result of md5Of, but this was the exact line that was causing me
 grief. Indeed, it looks innocent enough..
 
 So, toHexString returns a static array, which can be implicitly
 assigned to a member slice. In  safe code. I was horrified ;). Imo
 it's one of the most serious violations of D's safe by default
 principle.


Ouch!! Wow, that's really nasty. :-(  It totally went by me, even though
I've been bitten before by the variadic ctor bug.


T

-- 
Tech-savvy: euphemism for nerdy.
May 07 2014
parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Wednesday, 7 May 2014 at 20:09:22 UTC, H. S. Teoh via 
Digitalmars-d-learn wrote:

 On Wed, May 07, 2014 at 06:31:15PM +0000, Rene Zwanenburg via 
 Digitalmars-d-learn wrote:

 On Wednesday, 7 May 2014 at 15:41:19 UTC, Nick Sabalausky 
 wrote:

On 5/6/2014 6:46 PM, Rene Zwanenburg wrote:




 [...]

struct S
{
     safe:
    string str;

    this(string data)
    {
        import std.digest.md;
        str = md5Of(data).toHexString(); // Oops...
    }
}


That must be a terribly subtle one, I'm not seeing the 
problem at
all.

I get that md5Of returns a static array, and then a slice of 
it gets
passed to toHexString, but AIUI toHexString finishes (and 
returns a
newly allocated string) before the temporary static array 
leaves
scope.


 
 toHexString has an overload that takes a static array and can
 therefore return a static array (the length is known to be 
 twice the
 input length). In essence it's the same bug as directly 
 storing the
 result of md5Of, but this was the exact line that was causing 
 me
 grief. Indeed, it looks innocent enough..
 
 So, toHexString returns a static array, which can be implicitly
 assigned to a member slice. In  safe code. I was horrified ;). 
 Imo
 it's one of the most serious violations of D's safe by default
 principle.


 Ouch!! Wow, that's really nasty. :-(  It totally went by me, 
 even though
 I've been bitten before by the variadic ctor bug.


 T


FYI, I think this is one of the biggest implicit static 
array=>dynamic array bug you can do.

What's more, slicing of an rvalue static arrays is wrong 100% of 
the time. It's taking the address of a temporary. And the 
compiler should be able to catch it easy-peasy.

I filed this one:
https://issues.dlang.org/show_bug.cgi?id=12625
implicit slicing of RValue static array should be illegal

While I do (kinda) agree we can't deprecate static array to 
dynamic array implicit conversion, THIS is one case we should 
ban. It's *never* correct. Always a bug.
May 07 2014