dlang
Why does a static array accept incomplete initialisation? Please make that a warning.
immutable int[5] x = [1, 2, 3, 4, 5]; // <-- this compiles
immutable int[5] x = [1, 2, 3, 4, 5, 6]; // <-- this does not...
immutable int[5] x = [1, 2, 3, 4]; // <-- this compiles (!!!) excusemewtf?!
I'm sure this is un-changeable, but can we please get a warning for this? I reeeeeallly want this to be something that doesn't silently slip through the cracks!
It wouldn't be such a problem if we could write int[$] = [1,2,3,4];, but since we can't do that, this is almost certainly a bug at every location.
I just searched through my code for cases of this (I have a lot of tables), and found 3 actual real cases of actual bugs from this. (!)
How would it handle literals with an element index? Presumably allow missing elements in that case?
Any missing elements will be initialized to the default value of the element type.
int[3] a = [ 1:2, 3 ]; // a[0] = 0, a[1] = 2, a[2] = 3
Also, it seems to depend on whether the declaration is in a function:
immutable int[5] x = [1, 2, 3, 4]; // OK
void f(){
immutable int[5] x = [1, 2, 3, 4]; // Error
}
I think we need a way to suppress the proposed warning. I thought using 0: first would work, but it doesn't. It seems the current 'mismatched array lengths' error (in a function) is bypassed only if an indexed element actually skips an item:
void f()
{
int[3] a = [ 1:2, 3 ]; // OK
int[3] b = [ 0:1, 2 ]; // Error
int[3] c = [ 1, 1:2 ]; // Error
}
This behavior is inherited from C, and it is another case of D's special 'array initializers' vs expression initializers using array literals giving surprising behavior. Nick showed the inconsistency with function scopes, another oddity is:
immutable int[5] x = [1, 2, 3, 4]; // Allowed
immutable int[5] y = [1, 2, 3, 4] ~ []; // Error
Warnings are bad, such error prone initializations (see https://github.com/dlang/dmd/pull/21820) should always be an error, unless it's reasonably explicit that the incomplete initialization is intended. I say it's okay when indices are present or the literal is empty:
bool[256] isWhite = [' ': true, '\r': true, '\t': true, '\n': true];
ubyte[256] = [];
Otherwise it's a deprecation / error in next edition.
Great! An error is even better; I just assumed such a change would be impossible, because it will break the holy code in the wild.
Many arrays have the leftmost part initialized, and the rest gets default initialized. Larger arrays make it too tedious to require explicit initialization.
I don't approve of this change.
immutable int[5] y = [1, 2, 3, 4] ~ []; // Error
Not sure why that doesn't work.
Larger arrays make it too tedious to require explicit initialization.
Explicit initialization is not required, literals with indices still allow incomplete initialization, and in my implementation the error message suggests that solution making it discoverable. In dmd, druntime and Phobos, there has been 1 legit case and 4 bugs uncovered by my Pull Request. Small sample size, but 80% erroneous usage is clearly a problem, or do you want me to look at buildkite projects as well to collect more data?
If you don't accept making implicit incomplete array initialization an error as a solution, then what do you propose? Like Manu said, int[$] = [...] syntax would also mitigate this.
The
int[$]would be a better solution.
[$] would be glorious and it would mitigate some cases, but this is actually separate than that; sometimes you don't want an array to auto-adjust its length, you actually want it to be a particular length.
I still agree with what Dennis said above. I audited my code and found 3 more silent actual bugs as a result of this.
I have never encountered a case where it's made sense to init the "first few" of a fixed array; you either reset all of them to the same value int[N] x = 10;, or you want to init the array.
And like Dennis said; if you need selective initialisation, you can use the explicit indexed-initialisation.
An alternative expression would be to init the slice you intend;
int[10] x;
x[0..5] = [1,2,3,4,5]; // <- explicitly init the first 5 only
I would prefer that to be written in any code I review, because now there's no question whether the author wrote a bug or not.
Small sample size, but 80% erroneous usage is clearly a problem, or do you want me to look at buildkite projects as well to collect more data?
My own data found 3 more, so that's 4 errors in about 8 instances of this pattern; ~50% error rate! That's outrageous :P
I have never encountered a case where it's made sense to init the "first few" of a fixed array
I have, and I've done a few myself.
This is very long standing C and C++ practice. It's impractical to write out thousands of zeros.
I have never encountered a case where it's made sense to init the "first few" of a fixed array
I have, and I've done a few myself.
This is very long standing C and C++ practice. It's impractical to write out thousands of zeros.
Yes, it's especially common when you need a null terminator. We can also just use an explicit T() or T.init for that in D when we need it, though.
It's impractical to write out thousands of zeros.
Of course, which is why you can still use incomplete initialization by adding an index so there's no problem there.
Yes, it's especially common when you need a null terminator
Careful with that though, because char.init is 0xFF in D instead of 0. String initializers like char[8] x = "abc"; are a different thing than array initializers ['a', 'b', 'c'] though, and the former still puts zeros in the remainder.
Me and Dennis have both done an audit and found the error-rate is between 50-85%... cases of wrong-design are rarely more clear or egregious.
Without prior testing
enum xdata = [1, 2, 3, 4];
immutable int[xdata.length] x = xdata; //or slice
I just encountered another case of this issue in live code causing a nasty silent bug... I don't think this situation is defensible.
I made a patch to the compiler to give the error, and promptly ran into trouble:
dmd/compiler/src/dmd/backend/dwarfdbginf.d(327): Error: insufficient array initializers - has 9 elements, but needs 97
and several more in the same vein. Do I really want to add 97 zeros?
Initialize it to all zeros, then asign the 9 elements to a slice. That has the advantage of being really clear.
@WalterBright does it happen if you were to add explicit indexes?
0: { 0 },
1: { 0 },
...
int[105] x = 0; x[0..9] = [ a, few, values ];
You can't do this at module scope.
The array being initialized is a constant static, so doing it in two steps doesn't work.
As stated before, you only need to add an explicit index to still allow incomplete initialization. Check out my pull request, which passes all tests without adding 97 zeros to dwarfdbginf.d:
https://github.com/dlang/dmd/pull/21821
You could use some iota-like macro which repeats 0 n-times after the list of explicit initializations?
x = [ 1, 2, 3, repeat!(0, 97) ]
@dkorpel I looked over your PR, it's very good! Suggestions:
- I did not think of the index method, it definitely works
- It's on the "you have to really know the language" quirk list.
- The index method makes it work on older versions of the compiler. This is very good.
- Still, people will complain that their existing code breaks. Mine certainly did, in multiple places. I suggest making it a warning instead of a deprecation.
- It isn't clear from a non-expert D programmer what such code is supposed to do.
- Since editions are approved, can we just put this in an edition instead of the warning/deprecation?
- Definitely needs a spec PR and changelog
- In addition to your PR, I suggest supporting the following:
int[4] a = [1,2,...];
The ... is quite natural without needing to be puzzled out. It would be in addition to your idea.
Implementing ... is simple:
--- a/compiler/src/dmd/parse.d
+++ b/compiler/src/dmd/parse.d
@@ -6951,6 +6951,7 @@ class Parser(AST, Lexer = dmd.lexer.Lexer) : Lexer
*/
private AST.Initializer parseInitializer()
{
+ //printf("parseInitializer()\n");
const loc = token.loc;
switch (token.value)
@@ -7025,6 +7026,16 @@ class Parser(AST, Lexer = dmd.lexer.Lexer) : Lexer
commaExpected = true;
continue;
+ case TOK.dotDotDot:
+ if (commaExpected)
+ {
+ error("comma expected before `...]`");
+ }
+ ia.defaultInitialize = true;
+ nextToken();
+ check(TOK.rightBracket);
+ break;
+
case TOK.leftCurly:
case TOK.leftBracket:
if (commaExpected)