Thread overview
Confusing "Error: only one index allowed to index [...]"
Jan 02, 2019
Bastiaan Veelo
Jan 02, 2019
Bastiaan Veelo
Jan 02, 2019
Bastiaan Veelo
Jan 02, 2019
Dennis
Jan 03, 2019
Neia Neutuladh
Jan 03, 2019
Jonathan M Davis
Jan 03, 2019
Neia Neutuladh
January 02, 2019
I have been a bit confused by the fact that indexing syntax is opposite for two similar constructs, and that their error messages are very similar but misleading. The syntax is unfortunate but not easily fixable, my question is: Could and should we fix the error messages, and how.

The first case:
```
int[3][3] matrix;
matrix[1,1] = 7; // Error: only one index allowed to index `int[3][3]`
matrix[1][1] = 7; // OK
writeln(matrix[1]); // OK
```
What I find misleading is "only one index allowed" because `[1][1]` are two indices and clearly allowed. The message should neither be "only two indices allowed" as the last line illustrates. Fact is that `[1,1]` is the syntax applicable for when `opIndex` or `opIndexAssign` is defined, which isn't in this case (and can't be). So, maybe a better message would be: "no opIndexAssign defined for `int[3][3]`. Did you mean `[1][1]`?"

Now an example when `opIndexAssign` is defined (reduced):
```
struct A
{
	int[] _payload;
	alias _payload this;
	int opIndexAssign(int value, size_t i1, size_t i2) {return value;}
}

A a;
a[1,1] = 7;  // OK
a[1][1] = 7; // Error: only one index allowed to index `int`
```
Contrary to the first example, here `[1,1]` is the correct syntax. If instead mistakenly `[1][1]` is used, by means of the alias, the indices apply to `_payload`, which of course just takes one index. Ideally (but I imagine this would be difficult to implement) the compiler would recognise the presence of `opIndex` or `opIndexAssign` and ask "... Did you mean`[1,1]`?" Still, I think there is an error in the current message; it should either be "only one index allowed to index `int[]`" (notice the slice) or "cannot index `int`" (when the first `[1]` selects an element in `_payload` and the second `[1]` tries to index that).

Is it appropriate and feasible to change these messages to make it more obvious that the wrong syntax is being used?

(I know how to file issues, just wanted to air this here first.)

Thanks,
Bastiaan.
January 02, 2019
On 1/2/19 3:20 PM, Bastiaan Veelo wrote:
> I have been a bit confused by the fact that indexing syntax is opposite for two similar constructs, and that their error messages are very similar but misleading. The syntax is unfortunate but not easily fixable, my question is: Could and should we fix the error messages, and how.
> 
> The first case:
> ```
> int[3][3] matrix;
> matrix[1,1] = 7; // Error: only one index allowed to index `int[3][3]`
> matrix[1][1] = 7; // OK
> writeln(matrix[1]); // OK
> ```
> What I find misleading is "only one index allowed" because `[1][1]` are two indices and clearly allowed. The message should neither be "only two indices allowed" as the last line illustrates. Fact is that `[1,1]` is the syntax applicable for when `opIndex` or `opIndexAssign` is defined, which isn't in this case (and can't be). So, maybe a better message would be: "no opIndexAssign defined for `int[3][3]`. Did you mean `[1][1]`?"

I think this would be a good addition, and definitely possible, since it's a builtin. For custom types, I'm not sure it would work, but you probably don't get the same error.

> 
> Now an example when `opIndexAssign` is defined (reduced):
> ```
> struct A
> {
>      int[] _payload;
>      alias _payload this;
>      int opIndexAssign(int value, size_t i1, size_t i2) {return value;}
> }
> 
> A a;
> a[1,1] = 7;  // OK
> a[1][1] = 7; // Error: only one index allowed to index `int`
> ```
> Contrary to the first example, here `[1,1]` is the correct syntax. If instead mistakenly `[1][1]` is used, by means of the alias, the indices apply to `_payload`, which of course just takes one index. Ideally (but I imagine this would be difficult to implement) the compiler would recognise the presence of `opIndex` or `opIndexAssign` and ask "... Did you mean`[1,1]`?" Still, I think there is an error in the current message; it should either be "only one index allowed to index `int[]`" (notice the slice) or "cannot index `int`" (when the first `[1]` selects an element in `_payload` and the second `[1]` tries to index that).

In general, the fact that the compiler jumps right to trying the alias this for certain things, and ignores the original type is a problem. It comes up again and again. I don't think it's simply an indexing problem.

Remove the alias this and you get a MUCH better error:

Error: no [] operator overload for type A

In fact, you could say, for the first issue: "No [,] operator overload for type int[3][3]"

-Steve
January 02, 2019
On Wednesday, 2 January 2019 at 20:20:12 UTC, Bastiaan Veelo wrote:
> I have been a bit confused by the fact that indexing syntax is opposite for two similar constructs, and that their error messages are very similar but misleading.

I recently stumbled on this confusing error too and made an issue for the case of indexing integers:
https://issues.dlang.org/show_bug.cgi?id=19534
It seems like 'only one index allowed' is the compiler's go-to error when anything is wrong with indices.
January 02, 2019
On 1/2/19 5:08 PM, Steven Schveighoffer wrote:

> In general, the fact that the compiler jumps right to trying the alias this for certain things, and ignores the original type is a problem. It comes up again and again. I don't think it's simply an indexing problem.
> 

Actually, I take this back. This is the correct error.

What is happening is the compiler is evaluating expressions:

a[1] resolves just fine, because of the int[] alias this. But then you are trying to index the result, which is an int.

So no, this error cannot be improved, and my initial analysis is wrong -- the compiler is not not jumping to the alias this incorrectly.

-Steve
January 02, 2019
On Wednesday, 2 January 2019 at 22:49:45 UTC, Steven Schveighoffer wrote:
> What is happening is the compiler is evaluating expressions:
>
> a[1] resolves just fine, because of the int[] alias this. But then you are trying to index the result, which is an int.

Indeed, this is one of the things I described :-)

> So no, this error cannot be improved, [...]

I think yes, because the message is: “Error: only one index allowed to index `int`“ whereas it should be “Error: cannot index type `int`“. Right?

Bastiaan.
January 02, 2019
On Wednesday, 2 January 2019 at 23:37:17 UTC, Bastiaan Veelo wrote:
> On Wednesday, 2 January 2019 at 22:49:45 UTC, Steven Schveighoffer wrote:
>> What is happening is the compiler is evaluating expressions:
>>
>> a[1] resolves just fine, because of the int[] alias this. But then you are trying to index the result, which is an int.
>
> Indeed, this is one of the things I described :-)
>
>> So no, this error cannot be improved, [...]
>
> I think yes, because the message is: “Error: only one index allowed to index `int`“ whereas it should be “Error: cannot index type `int`“. Right?
>
> Bastiaan.

Which is the bug that Dennis reported. https://issues.dlang.org/show_bug.cgi?id=19534
January 03, 2019
On Wed, 02 Jan 2019 20:20:12 +0000, Bastiaan Veelo wrote:
> What I find misleading is "only one index allowed" because `[1][1]` are two indices and clearly allowed.

int[3][3] only allows one index. The value this yields is an int[3], which in turn only allows one index. But that's cutting hairs pretty fine.
January 02, 2019
On Wednesday, January 2, 2019 6:31:31 PM MST Neia Neutuladh via Digitalmars- d wrote:
> On Wed, 02 Jan 2019 20:20:12 +0000, Bastiaan Veelo wrote:
> > What I find misleading is "only one index allowed" because `[1][1]` are two indices and clearly allowed.
>
> int[3][3] only allows one index. The value this yields is an int[3], which in turn only allows one index. But that's cutting hairs pretty fine.

As with many error messages, the message is very much correct, but some folks may find in confusing. Given that error messages tend to be written from the standpoint of what the compiler sees rather than from what the programmer meant, it can be a bit difficult to write good error messages that really help a confused developer figure out what's wrong with their code. But creating enhancement requests in bugzilla whenever you find an error message confusing can lead to improved error messages. Without that, it's unlikely that such messages will ever be improved.

- Jonathan M Davis



January 03, 2019
On Wed, 02 Jan 2019 19:25:57 -0700, Jonathan M Davis wrote:
> As with many error messages, the message is very much correct, but some folks may find in confusing. Given that error messages tend to be written from the standpoint of what the compiler sees rather than from what the programmer meant, it can be a bit difficult to write good error messages that really help a confused developer figure out what's wrong with their code. But creating enhancement requests in bugzilla whenever you find an error message confusing can lead to improved error messages. Without that, it's unlikely that such messages will ever be improved.

And one of the great things about improved error messages is that they don't need much ceremony compared to other PR requests. No DIPs, no compiler flags, no transition plan.
January 04, 2019
On 1/2/19 6:37 PM, Bastiaan Veelo wrote:
> On Wednesday, 2 January 2019 at 22:49:45 UTC, Steven Schveighoffer wrote:
>> What is happening is the compiler is evaluating expressions:
>>
>> a[1] resolves just fine, because of the int[] alias this. But then you are trying to index the result, which is an int.
> 
> Indeed, this is one of the things I described :-)
> 
>> So no, this error cannot be improved, [...]
> 
> I think yes, because the message is: “Error: only one index allowed to index `int`“ whereas it should be “Error: cannot index type `int`“. Right?

Yes, that error is super-misleading. I should have narrowed my focus of my retraction, I take it back that the alias this is a problem.

-Steve