www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Confusing "Error: only one index allowed to index [...]"

reply Bastiaan Veelo <Bastiaan Veelo.net> writes:
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.
Jan 02 2019
next sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
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
Jan 02 2019
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
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
Jan 02 2019
parent reply Bastiaan Veelo <Bastiaan Veelo.net> writes:
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.
Jan 02 2019
next sibling parent Bastiaan Veelo <Bastiaan Veelo.net> writes:
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
Jan 02 2019
prev sibling parent Steven Schveighoffer <schveiguy gmail.com> writes:
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
Jan 04 2019
prev sibling next sibling parent Dennis <dkorpel gmail.com> writes:
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.
Jan 02 2019
prev sibling parent reply Neia Neutuladh <neia ikeran.org> writes:
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.
Jan 02 2019
parent reply Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
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
Jan 02 2019
parent Neia Neutuladh <neia ikeran.org> writes:
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.
Jan 02 2019