Bug 12420 – [AA] Can't set associative array with array as key value using key type

Status
REOPENED
Severity
regression
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2014-03-20T03:02:44Z
Last change time
2024-12-13T18:18:28Z
Keywords
pull
Assigned to
No Owner
Creator
Denis Shelomovskii
Moved to GitHub: dmd#17648 →

Comments

Comment #0 by verylonglogin.reg — 2014-03-20T03:02:44Z
This code should compile: --- void main() { int[char[]] aa; aa[new char[1]] = 5; // line 4 } --- main.d(4): Error: associative arrays can only be assigned values with immutable keys, not char[] --- Also the error message is wrong. The compiler expects an array of not-mutable elements (yes, `cast(const)` will work). This incorrect compiler hack [1] was introduced trying to fix Issue 2954 (see Comment 8 of that issue). I propose to remove the hack. And yes, it's a regression from 2010. ) [1] http://www.dsource.org/projects/dmd/changeset/749
Comment #1 by k.hara.pg — 2014-03-29T20:21:09Z
(In reply to comment #0) > This code should compile: > --- > void main() > { > int[char[]] aa; > aa[new char[1]] = 5; // line 4 > } > --- > main.d(4): Error: associative arrays can only be assigned values with immutable > keys, not char[] > --- I agree with the conclusion, the reason in my thought is: `new char[1]` makes unique data and have no foreign reference, so it can be implicitly convertible to immutable(char[]). Therefore directly using it in the index part of the AA element setting will be allowed.
Comment #2 by k.hara.pg — 2014-03-29T20:51:15Z
Comment #3 by verylonglogin.reg — 2014-03-30T01:27:43Z
(In reply to comment #1) > I agree with the conclusion, the reason in my thought is: > > `new char[1]` makes unique data and have no foreign reference, so it can be > implicitly convertible to immutable(char[]). Therefore directly using it in the > index part of the AA element setting will be allowed. Looks like you misunderstood me. The word `immutable` shouldn't be here, it's just an incorrect error message. Currently we have no rules disallowing non-`immutable` associative array keys. I filed Issue 12491 for this. This issue will be auto-fixed if Issue 2954 will be fixed by applying a correct check to ensure key expression can be implicitly converted to AA key type in case of setting AA value.
Comment #4 by k.hara.pg — 2014-06-21T08:23:35Z
(In reply to Denis Shelomovskij from comment #3) > Looks like you misunderstood me. The word `immutable` shouldn't be here, > it's just an incorrect error message. Currently we have no rules disallowing > non-`immutable` associative array keys. I filed Issue 12491 for this. > > This issue will be auto-fixed if Issue 2954 will be fixed by applying a > correct check to ensure key expression can be implicitly converted to AA key > type in case of setting AA value. In D, AA requires immutability for the key. In evidence all AA key types are implicitly qualified with const. int[char[]] aa; pragma(msg, typeof(aa)); // Prints: int[const(char)[]] Honesty, key type should be qualified with immutable. Currently int[const(char[])] can accept char[] key when storing value in the AA, but it's a known issue. Anyway in D AA types cannot have mutable key. Therefore this issue should be marked as invalid.(In reply to Denis Shelomovskij from comment #0) > This code should compile: > --- > void main() > { > int[char[]] aa; > aa[new char[1]] = 5; // line 4 > } > --- > main.d(4): Error: associative arrays can only be assigned values with > immutable keys, not char[] > --- > > Also the error message is wrong. The compiler expects an array of > not-mutable elements (yes, `cast(const)` will work). This incorrect compiler > hack [1] was introduced trying to fix Issue 2954 (see Comment 8 of that > issue). I propose to remove the hack. > > And yes, it's a regression from 2010. ) > > > [1] http://www.dsource.org/projects/dmd/changeset/749
Comment #5 by k.hara.pg — 2014-06-21T08:24:39Z
(Sorry, ignore my comment #4.) (In reply to Denis Shelomovskij from comment #3) > Looks like you misunderstood me. The word `immutable` shouldn't be here, > it's just an incorrect error message. Currently we have no rules disallowing > non-`immutable` associative array keys. I filed Issue 12491 for this. > > This issue will be auto-fixed if Issue 2954 will be fixed by applying a > correct check to ensure key expression can be implicitly converted to AA key > type in case of setting AA value. In D, AA requires immutability for the key. In evidence all AA key types are implicitly qualified with const. int[char[]] aa; pragma(msg, typeof(aa)); // Prints: int[const(char)[]] Honesty, key type should be qualified with immutable. Currently int[const(char[])] can accept char[] key when storing value in the AA, but it's a known issue. Anyway in D AA types cannot have mutable key. Therefore this issue should be marked as invalid.
Comment #6 by verylonglogin.reg — 2015-01-02T16:37:09Z
(In reply to Kenji Hara from comment #5) > In D, AA requires immutability for the key. Not now. Currently it just adds `const` qualifier. This is another Issue 12491. > Anyway in D AA types cannot have mutable key. Therefore this issue should be > marked as invalid. This issue should not be marked as invalid as it results in such nonsense compiler behaviour as `char[]` is implicitly convertible to `const char[]`: --- void main() { int[char[]] aa; auto c1 = new char[1]; aa[c1] = 5; // error const c2 = c1; aa[c2] = 5; // ok } ---
Comment #7 by code — 2015-03-10T15:59:08Z
So what's the conclusion here?
Comment #8 by k.hara.pg — 2015-03-10T16:12:58Z
(In reply to Martin Nowak from comment #7) > So what's the conclusion here? Denis argues that mutable key char[] should be allowed for the AA indexing. But AA key should be const on indexing, and should be immutable on setting. Therefore specifying mutable key char[] is not possible. (I'm saying if and only if a NewExpression `new char[1]` is directly used, it can be convertible to immutable(char[]) by the uniqueness. But it's different from his intention.) Therefore this issue should be closed as invalid.
Comment #9 by verylonglogin.reg — 2015-03-10T17:21:24Z
(In reply to Kenji Hara from comment #8) > (In reply to Martin Nowak from comment #7) > > So what's the conclusion here? > > Denis argues that mutable key char[] should be allowed for the AA indexing. > But AA key should be const on indexing, and should be immutable on setting. > Therefore specifying mutable key char[] is not possible. Everything I want is to clarify AA behviour (emn... no, really I'd like to remove this from a language as a terrible misfeature, but I'm almost alone here so I do not propose it). Regarging to types accepted by AAs I'd like Issue 12491 & Issue 12492 to be discussed and resolved some way. > (I'm saying if and only if a NewExpression `new char[1]` is directly used, > it can be convertible to immutable(char[]) by the uniqueness. But it's > different from his intention.) This issue isn't connected with `immutable` at all. As I wrote in description it's just a misleading error mesage. Go to enhancement Issue 12491 for AA and immutability discussion. > Therefore this issue should be closed as invalid. I can't believe anyone can accept bahaviour shown in Comment 6.
Comment #10 by schveiguy — 2015-03-10T18:47:37Z
(In reply to Denis Shelomovskij from comment #9) > I can't believe anyone can accept bahaviour shown in Comment 6. I agree with you, and I have argued as much in the NG. The compiler should either enforce immutability, or not care. Enforcing const has all of the annoyance, with none of the guarantee. However, practicality dictates that we cannot force immutability on everyone who wants to use an AA. In particular, immutable classes are nearly useless. I would like to see AA's remove the const enforcement. I'm not sure how I feel about immutable enforcement, but it would be a nice option to add once AA's are fully library types.
Comment #11 by verylonglogin.reg — 2015-03-10T19:06:12Z
(In reply to Steven Schveighoffer from comment #10) > (In reply to Denis Shelomovskij from comment #9) > > > I can't believe anyone can accept bahaviour shown in Comment 6. > > I agree with you, and I have argued as much in the NG. The compiler should > either enforce immutability, or not care. Enforcing const has all of the > annoyance, with none of the guarantee. But htis issue even not about this. AA's key type is `const char[]` and `char[]` is implicitly convertible to `const char[]` anyway. So this issue is about error in type checking and error in error message. )
Comment #12 by schveiguy — 2015-03-10T19:30:21Z
(In reply to Denis Shelomovskij from comment #11) > > But htis issue even not about this. AA's key type is `const char[]` and > `char[]` is implicitly convertible to `const char[]` anyway. So this issue > is about error in type checking and error in error message. ) OK, I didn't notice that aspect, yes that is even more perplexing than the issue of requiring const. In fact, a library-based type would solve that too :)
Comment #13 by bugzilla — 2015-07-26T08:22:32Z
The error message is correct. It's really not safe to have AA keys that are mutable. Yes, the compiler currently accepts const keys, and const keys are mutable by other references. But that is the bug, not the error message about immutability. Kenji is also correct that a Unique reference, even if it is mutable, should also be accepted. This can be implemented in the compiler by testing to see if a mutable reference can be implicitly converted to mutable.
Comment #14 by bugzilla — 2015-07-26T08:41:56Z
Comment #15 by verylonglogin.reg — 2015-07-26T10:20:52Z
(In reply to Walter Bright from comment #13) > ... > Yes, the compiler currently accepts const keys, and const keys are mutable > by other references. But that is the bug, not the error message about > immutability. > ... If accepting not implicitly convertible to `immutable` types as associative array keys is a bug, please approve Issue 12491.
Comment #16 by dlang-bot — 2020-02-12T02:27:09Z
@WalterBright updated dlang/dmd pull request #4835 "[REG] fix Issue 12420 - [AA] Can't set associative array with array as key …" fixing this issue: - fix Issue 12420 - [AA] Can't set associative array with array as key value using key type https://github.com/dlang/dmd/pull/4835
Comment #17 by robert.schadek — 2024-12-13T18:18:28Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/17648 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB