seems T[char[]] is rewritten as T[const(char)[]], and does not accept char[] as key even if mutable data should automatically convert to const (right..?)

Shouldn't T[char[]] be disallowed, and have to be written as T[immutable(char)[]] instead of a silent rewrite?


    alias long[char[]] AA;
    // key automatically changed to const(char)[]
    static assert(is(AA == long[const(char)[]]));
    AA aa;
    aa["a"] = 10;
    // error - have to use immutable keys
    aa["b".dup] = 11;

simendsjo:

> Shouldn't T[char[]] be disallowed, and have to be written as T[immutable(char)[]] instead of a silent rewrite?

Of course. I have a Bugzilla issue on this.

Bye,
bearophile

On Tue, Jan 3, 2012 at 1:25 PM, simendsjo <simendsjo@gmail.com> wrote:
> seems T[char[]] is rewritten as T[const(char)[]], and does not accept char[]
> as key even if mutable data should automatically convert to const (right..?)
>
> Shouldn't T[char[]] be disallowed, and have to be written as T[immutable(char)[]] instead of a silent rewrite?
>
>
>    alias long[char[]] AA;
>    // key automatically changed to const(char)[]
>    static assert(is(AA == long[const(char)[]]));
>    AA aa;
>    aa["a"] = 10;
>    // error - have to use immutable keys
>    aa["b".dup] = 11;

By design, the problem is things like this:

char[] key = "somekey";
long[char[]] aa;
aa[key] = 5;
key[2] = 'b';

If this were allowed, the associative array would reach an invalid state where the hash it stored for the key is no longer correct.

It does seem like T[char[]] should be disallowed and the requirement for immutable keys should be literally enforced, and it's possible that was intended but immutable/const weren't as complete when this problem was last visited. I'll see if I can dig up an old discussion about this.

On Tue, Jan 3, 2012 at 1:41 PM, Andrew Wiley <wiley.andrew.j@gmail.com> wrote:
> On Tue, Jan 3, 2012 at 1:25 PM, simendsjo <simendsjo@gmail.com> wrote:
>> seems T[char[]] is rewritten as T[const(char)[]], and does not accept char[]
>> as key even if mutable data should automatically convert to const (right..?)
>>
>> Shouldn't T[char[]] be disallowed, and have to be written as T[immutable(char)[]] instead of a silent rewrite?
>>
>>
>>    alias long[char[]] AA;
>>    // key automatically changed to const(char)[]
>>    static assert(is(AA == long[const(char)[]]));
>>    AA aa;
>>    aa["a"] = 10;
>>    // error - have to use immutable keys
>>    aa["b".dup] = 11;
>
> By design, the problem is things like this:
>
> char[] key = "somekey";

Sorry, should be:
char[] key = "somekey".dup;

On 03.01.2012 20:41, Andrew Wiley wrote:
> On Tue, Jan 3, 2012 at 1:25 PM, simendsjo<simendsjo@gmail.com>  wrote:
>> seems T[char[]] is rewritten as T[const(char)[]], and does not accept char[]
>> as key even if mutable data should automatically convert to const (right...?)
>>
>> Shouldn't T[char[]] be disallowed, and have to be written as
>> T[immutable(char)[]] instead of a silent rewrite?
>>
>>
>>     alias long[char[]] AA;
>>     // key automatically changed to const(char)[]
>>     static assert(is(AA == long[const(char)[]]));
>>     AA aa;
>>     aa["a"] = 10;
>>     // error - have to use immutable keys
>>     aa["b".dup] = 11;
>
> By design, the problem is things like this:
>
> char[] key = "somekey";
> long[char[]] aa;
> aa[key] = 5;
> key[2] = 'b';
>
> If this were allowed, the associative array would reach an invalid
> state where the hash it stored for the key is no longer correct.
>
> It does seem like T[char[]] should be disallowed and the requirement
> for immutable keys should be literally enforced, and it's possible
> that was intended but immutable/const weren't as complete when this
> problem was last visited. I'll see if I can dig up an old discussion
> about this.

It is disallowed, but it's enforced when setting a key rather than when constructing the type.
So `aa[key] = 5` above fails as key is char[] rather than string.

On Tue, Jan 3, 2012 at 1:50 PM, simendsjo <simendsjo@gmail.com> wrote:
> On 03.01.2012 20:41, Andrew Wiley wrote:
>>
>> On Tue, Jan 3, 2012 at 1:25 PM, simendsjo<simendsjo@gmail.com>  wrote:
>>>
>>> seems T[char[]] is rewritten as T[const(char)[]], and does not accept
>>> char[]
>>> as key even if mutable data should automatically convert to const
>>> (right...?)
>>>
>>>
>>> Shouldn't T[char[]] be disallowed, and have to be written as T[immutable(char)[]] instead of a silent rewrite?
>>>
>>>
>>>    alias long[char[]] AA;
>>>    // key automatically changed to const(char)[]
>>>    static assert(is(AA == long[const(char)[]]));
>>>    AA aa;
>>>    aa["a"] = 10;
>>>    // error - have to use immutable keys
>>>    aa["b".dup] = 11;
>>
>>
>> By design, the problem is things like this:
>>
>> char[] key = "somekey";
>> long[char[]] aa;
>> aa[key] = 5;
>> key[2] = 'b';
>>
>> If this were allowed, the associative array would reach an invalid state where the hash it stored for the key is no longer correct.
>>
>> It does seem like T[char[]] should be disallowed and the requirement for immutable keys should be literally enforced, and it's possible that was intended but immutable/const weren't as complete when this problem was last visited. I'll see if I can dig up an old discussion about this.
>
>
> It is disallowed, but it's enforced when setting a key rather than when
> constructing the type.
> So `aa[key] = 5` above fails as key is char[] rather than string.

Yes, while that's correct, it doesn't make much sense (as you pointed
out). Rewriting long[char[]] to long[const(char)[]] isn't particularly
useful when you can't use char[] as a key.
If the compiler is basically going to disallow using the AA as
anything but a long[string], it should really disallow declaring
anything with a mutable key type. Disallowing mutable keys at that
assignment site but allowing them in the type is confusing.

Andrew Wiley:

> If the compiler is basically going to disallow using the AA as anything but a long[string], it should really disallow declaring anything with a mutable key type. Disallowing mutable keys at that assignment site but allowing them in the type is confusing.

Two related bug reports: http://d.puremagic.com/issues/show_bug.cgi?id=4475 http://d.puremagic.com/issues/show_bug.cgi?id=6253

Bye,
bearophile

On Tue, Jan 3, 2012 at 4:20 PM, bearophile <bearophileHUGS@lycos.com> wrote:
> Andrew Wiley:
>
>> If the compiler is basically going to disallow using the AA as anything but a long[string], it should really disallow declaring anything with a mutable key type. Disallowing mutable keys at that assignment site but allowing them in the type is confusing.
>
> Two related bug reports: http://d.puremagic.com/issues/show_bug.cgi?id=4475
"Improving the compiler 'in' associative array can return just a bool" Whether this is a good idea or not is a moot point. Changing this would break too much code (basically all code that uses AAs significantly).

> http://d.puremagic.com/issues/show_bug.cgi?id=6253
"Refuse definition too of impossible associative arrays"

This is what we're discussing.
Some consequences of actually changing this:
- This breaks D1 compatibility of AAs across the board because
immutable simply didn't exist then
- Significant D2 code breakage as well

We might see if Walter is willing to add this as a warning and/or deprecation to see whether it's actually feasible to disallow it completely.

Andrew Wiley:

> Some consequences of actually changing this:
> - This breaks D1 compatibility of AAs across the board because
> immutable simply didn't exist then

D1 compatibility will stop being a problem in some time :-)


> - Significant D2 code breakage as well

It's essentially wrong code, because you are declaring something you can't actually use, so "breaking it" is an improvement.

Bye,
bearophile