Thread overview
[Issue 13670] bug in assigning to dynamic array element
Nov 02, 2014
Jonathan M Davis
Nov 02, 2014
Ketmar Dark
Nov 02, 2014
Jonathan M Davis
Nov 02, 2014
Ketmar Dark
Nov 02, 2014
Jonathan M Davis
Nov 02, 2014
Ketmar Dark
Nov 29, 2014
Walter Bright
Nov 29, 2014
Ketmar Dark
Nov 29, 2014
Ketmar Dark
November 02, 2014
https://issues.dlang.org/show_bug.cgi?id=13670

Jonathan M Davis <issues.dlang@jmdavisProg.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |issues.dlang@jmdavisProg.co
                   |                            |m

--- Comment #1 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
So, essentially, you want to do something like

a[i] = i++

and have that be equivalent to

i++;
a[i] = i;

In C++,

a[i] = i++;

the order of evaluation is undefined, so it results in undefined behavior. AFAIK, this is also the case in D. Making it so that the order of evaluation is always left-to-right has been discussed, but AFAIK, it's not actually what the language currently does or the spec requires. It may be that that will change in the future, but I don't know.

Regardless, I'd advise that you avoid any code where you mutate a variable in an expression and then use that same variable elsewhere in the expression. Even if the order of evaluation does get defined in D at some point in the future, it won't be in other languages, so getting in the habit of relying on it is probably a bad idea.

In any case, AFAIK, your example does not currently count as a bug.

--
November 02, 2014
https://issues.dlang.org/show_bug.cgi?id=13670

Ketmar Dark <ketmar@ketmar.no-ip.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ketmar@ketmar.no-ip.org

--- Comment #2 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
(In reply to Jonathan M Davis from comment #1)
> So, essentially, you want to do something like
> 
> a[i] = i++
> 
> and have that be equivalent to
> 
> i++;
> a[i] = i;
nope. what i want for dynamic arrays is:
tmparr = &a;
tmpidx = i*a[0].sizeof;
i++;
tmparr.ptr[tmpidx] = i;

there is nothing that breaks evaluation order in case of dynamic arrays, and with this change they will work exactly the same as static arrays.

or at least make compiler emit error on such assignments, so i don't have to track if my variable is dynamic or static array manually.

> Regardless, I'd advise that you avoid any code where you mutate a variable in an expression and then use that same variable elsewhere in the expression.
as i wrote in NG, current behavior with dynamic arrays is plainly wrong. it breaks "the least surprise" principle and requires programmer to manually track variable types. not something i'd expect from "safe and powerful" language.

> In any case, AFAIK, your example does not currently count as a bug.
ah. so close it, please.

--
November 02, 2014
https://issues.dlang.org/show_bug.cgi?id=13670

Jonathan M Davis <issues.dlang@jmdavisProg.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |INVALID

--- Comment #3 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
(In reply to Ketmar Dark from comment #2)
> so i don't have to
> track if my variable is dynamic or static array manually.

In general, I'd be very worried about code that didn't care about whether it was dealing with a dynamic array or a static array, because they're fundamentally different types. It just seems like it's begging for trouble. The only case where that might make sense is where you have a dynamic array which is a slice of a static one, but then you're dealing with a dynamic one, not a static one, so you're still just dealing with dynamic arrays rather than having to worry about whether an array is dynamic or static.

--
November 02, 2014
https://issues.dlang.org/show_bug.cgi?id=13670

--- Comment #4 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
(In reply to Jonathan M Davis from comment #3)
> (In reply to Ketmar Dark from comment #2)
> > so i don't have to
> > track if my variable is dynamic or static array manually.
> 
> In general, I'd be very worried about code that didn't care about whether it was dealing with a dynamic array or a static array, because they're fundamentally different types.
so why so many efforts were spent to make dynamic arrays looks like static arrays? if they are "fundamentally different types", they shouldn't look the same. there must be another different operation for them with different syntax, different declaration syntax and so on. and compiler *must* at least warn me that it's so stupid that it can't make the simple code works as any sane person expected it to work.

and i still can't understand why i should manually track if my variable is of static array type or of dynamic array type. and i'm still sure that this code should never asserts, it's nonsense:

  info.list[idx] = saveIt(info, count-1);
  assert(info.list[idx] != 0);

it's insane. it's counterintuitive. it's beyoud any logic. language that asserts here for built-in type is a toy language which shouldn't be used even for simple throw-away scripts. i can never explain to anyone why built-in type have to work like this. bwah! i can't even explain this to myself. this is ridiculous.

D smells. with all that "let's keep our legacy 'cause Random Reddit User will go insane despite the actual users are begging for fix that", "let's not bless dfix for another year", etc.

another hobbyst project that wastes years of my time. my bad, i have to recognize that hobbyst smell from the beginning and not hoping for the best. i'm glad that i was unsuccessfull pushing our development department towards D. they were right, there is no sense to learn another quirky language that has so little libraries when we already have quirky C++ with alot of libraries.

so be it.

--
November 02, 2014
https://issues.dlang.org/show_bug.cgi?id=13670

--- Comment #5 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
(In reply to Ketmar Dark from comment #4)
> (In reply to Jonathan M Davis from comment #3)
> > (In reply to Ketmar Dark from comment #2)
> > > so i don't have to
> > > track if my variable is dynamic or static array manually.
> > 
> > In general, I'd be very worried about code that didn't care about whether it was dealing with a dynamic array or a static array, because they're fundamentally different types.
> so why so many efforts were spent to make dynamic arrays looks like static arrays? if they are "fundamentally different types", they shouldn't look the same. there must be another different operation for them with different syntax, different declaration syntax and so on. and compiler *must* at least warn me that it's so stupid that it can't make the simple code works as any sane person expected it to work.
> 
> and i still can't understand why i should manually track if my variable is of static array type or of dynamic array type. and i'm still sure that this code should never asserts, it's nonsense:

Static arrays are value types, whereas dynamic arrays are pseudo-reference types. So yes, they're fundamentally different, and it's due to how their memory is managed. There are some cases where it doesn't matter, but there are others where it does, and some operations are outright illegal on a static array that are perfectly legal on a dynamic one (e.g. assigning to length or appending). Regardless, there are no static arrays in your code, so I don't see why you're worried about telling the difference between a static array and a dynamic array in the context of this issue.

>   info.list[idx] = saveIt(info, count-1);
>   assert(info.list[idx] != 0);

It's failing because what you're doing on the right-hand side is mutating what you're assigning to. It doesn't matter if arrays are involved or not. You're going to run into trouble when you do that regardless of the type. Granted, because it's hidden in the call to saveIt rather than being right there in the expression, it's harder to catch, but that doesn't change the fact that it's undefined behavior. There have been discussions about making it defined behavior, in which case, it would probably be the case that you could rely on info.list[idx] being evaluated before the call to saveIt, but even that could be a funny, because what you're doing in the call to saveIt could cause info.list to be reallocated, meaning that the assignment would be made to the memory that the array referred to previously rather than after the call.

Mutating something at the same time that you're trying to assign to it just begging for trouble, and even with the order of evalution defined, I think that if the assignment is to a dynamic array, and you're doing anything in the mutation which could cause the memory for the array to be reallocated, then you're still going to have problems, because the memory backing the array could be reallocated in the middle of the expression. You're trying to do something that fundamentally is going to have problems.

> D smells. with all that "let's keep our legacy 'cause Random Reddit User will go insane despite the actual users are begging for fix that", "let's not bless dfix for another year", etc.
> 
> another hobbyst project that wastes years of my time. my bad, i have to recognize that hobbyst smell from the beginning and not hoping for the best. i'm glad that i was unsuccessfull pushing our development department towards D. they were right, there is no sense to learn another quirky language that has so little libraries when we already have quirky C++ with alot of libraries.

No offense, but you seem to blow up in discussions very quickly, which is not at all conducive to having an intelligent conversation. If you're looking for perfection in any language, you will be forever disappointed. Some of D's problems can and will be fixed. Others are intrinsic to how things work, and others might be nice to fix but won't be for various reasons (including things like fixing the problem not being worth the code breakage that the fix would cause).

If you want to point out things in D that you consider to be a problem and think should be fixed, then great. Have at it. But don't expect that everyone will agree with you or that things will change just because you want them to. Some of the time, what you want to happen may very well happen (especially if a bunch of other folks agree with you), but there are plenty of times where it won't change. The same goes for ideas or complaints that any of the rest of us have.

--
November 02, 2014
https://issues.dlang.org/show_bug.cgi?id=13670

--- Comment #6 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
i'm not looking for perfection, i'm looking merely for sanity. dynamic arrays should either work as static arrays, or not looking like static arrays. or compiler at least must to reject such code, as this code is obviously meaningless.

i'm accepting alot of things in D. while i can argue that "@safe" and "nothrow" is nonsence too (do either "@safe" and "@nothrow", or "safe" and "nothrow"), or that prefix "const" in function declaration should be forbidden, that doesn't makes me to throw D away. it's the things i can live with, albeit not very happy. ;-)

but this case is completely different. compiler behavior clearly violates the principle of least astonishment here. what it does is absolutely insane. really, no sane person would want this code to modify the stale copy of the array! and this is easily fixable — just postpone .ptr loading.

why, in heaven's sake, somebody will want the current behavior? it's useless. it's fixable. fixing it doesn't need to change evaluation order rules. what fix will make is just "sanification".

ok, if such fix is completely unacceptable, i can live with that too. but *only* if compiler will reject such code. this code is heavily error-prone. if list was a static array, and then was changed to dynamic one… everything will compile as before, but misteriously failing in production. and not with segfault, which is at least traceable, but with processing of invalid values.

this is what i can't call "safe". this is not just unsafe, this is completely undetectable without manual code checking! so compiler not only avoids using it's type checking system to help me write better code, it carefully protecting my invalid code with it! that is what i can't accept.

dynamic arrays are built-in type. compiler is perfectly able to generate error on the code like this which can cause side effects. just forbid it and insist on "pure" assigns for dynamic arrays. i bet such change will uncover some hidden problematic code in people's codebases (and maybe even in runtime/phobos too).

but doing nothing with that just makes the language unsafe for no reason. and if i have to use unsafe language anyway, i'd better go with C++. C++ has alot of code written, it has no problems interfacing both C and C++ code, and it's quirks are well-known.

i can live with things i don't like in D, but i can't live with features that not only hides problematic code, but tries to do their best to not tell me that i may doing something wrong. i'd better abandon such a language before i get a significant codebase written with it.

i will really miss D great metaprogramming abilities and it's great template system. but i don't want to use the language that tries to cheat me without even warning. we already have such languages, having one more is pointless.

and this really hurts me, 'cause i love D with passion, this is the only language (in it's class) that makes me feel like it cooperates with me to help me achieve my needs. maybe i'm overreacting, but i feel this like a betrayal from an old trusted friend. it hurts.

--
November 29, 2014
https://issues.dlang.org/show_bug.cgi?id=13670

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com

--- Comment #7 from Walter Bright <bugzilla@digitalmars.com> ---
The code is clearly depending on the order of evaluation of a=b, i.e. whether a or b is evaluated first. This is implementation defined in D (as it is in C and C++). My old C/C++ spidey sense goes on red alert, though, when I see code like this. (I still have a hard time figuring out what it's even supposed to do!)

It is not actually invalid code - the old array is still there and may have valid references to it.

The issue has nothing to do with static arrays, as static arrays cannot be resized.

There thing is, if D does define the order of execution of these, either L-R or R-L, it still may not do what a particular programmer expects, and should be cause for suspicion in a code review. I think such dependency should be avoided as a matter of style.

The workaround you tried is what the code should be, simply to make its intent clear.

There's a lot of merit in your suggestion that such dependency should be flagged as an error, though it won't be detectable in all cases:

   *foo() = *bar();

What if foo() and bar() have dependent side effects? The compiler can't tell in
all cases.

--
November 29, 2014
https://issues.dlang.org/show_bug.cgi?id=13670

--- Comment #8 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
the code is clearly meaningless in most cases. what compiler *should* do is at least emit warning about possible side-effects here, but not silently hiding that.

this is *built-in* type, i can't fix it by fixing my "dynamic array class". and i don't want to remember if the array i used is static or not, if the function changes it's length or not and so on. i want compiler to help me by emiting warning about such dangerous assigns. compiler *knows* that this is dynamic array, and it *knows* that the function is not marked as "pure", so it *must* warn me. not trying to cheat me, not playing games "are you smart enough?" with me, but help me to fix my code.

yet compiler says me nothing and hapily accepts this. ok, the program is not segfaulting, but i prefer it to segfault instead of silently modifying a stale array copy.

i was not expected such behavior with *built-in* types.

sure, compiler can't catch everything, but does this mean that it shouldn't try to catch at least something?

i catched at least three such bugs in my code, and all of them due to dynamic arrays that looking *exactly* as static arrays, so i wrote that code without second thought: "if it looks the same, it probably works the same, and if i wrong the compiler will tell me, right?" ah… wrong.

and then this was closed as "invalid", indicating that compiler will not stop it's attempts to cheat me and hide my bugs.

i'm not expecting that our juniors will not reproduce this bug over and over again, so "it's dangerous to go alone".

--
November 29, 2014
https://issues.dlang.org/show_bug.cgi?id=13670

--- Comment #9 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
p.s. this code is meaningless as it is, as it was written just to demonstrate the behavior. but in the real software that was the part of the module that creates GFF files for BioWare's Aurora Engine, and it has sense there.

--