Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misleading error messages when trying to use nonexistant []= #1490

Open
DiThi opened this issue Jan 19, 2025 · 4 comments
Open

Misleading error messages when trying to use nonexistant []= #1490

DiThi opened this issue Jan 19, 2025 · 4 comments
Labels
bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler good first issue Easy-to-start-with issue with no in-depth knowledge or complex implementation required.

Comments

@DiThi
Copy link

DiThi commented Jan 19, 2025

Example

type X = object
template `[]`(x: X, i: int) = 0
var x: X
x[0] = 1

Actual Output

bug.nim(5, 2) Error: type mismatch: got <X, int literal(0)>
but expected one of:
proc `[]`(s: string; i: BackwardsIndex): char
  first type mismatch at position: 0
      [ ... many examples later ... ]
template `[]`(x: X; i: int)
  first type mismatch at position: 0

expression: `[]`(x, 0)

Expected Output

bug.nim(5, 2) Error: type mismatch: got <X, int literal(0), int literal(1)>
but expected one of:
proc `[]=`(s: var string; i: BackwardsIndex; x: char)
  first type mismatch at position: 0
      [ ... many examples later ... ]

expression: `[]=`(x, 0, 1)

I was super confused because my [] template was appearing there, but didn't realize it was missing the []= and not [].

@saem saem added bug Something isn't working good first issue Easy-to-start-with issue with no in-depth knowledge or complex implementation required. compiler/sem Related to semantic-analysis system of the compiler labels Jan 19, 2025
@saem
Copy link
Collaborator

saem commented Jan 19, 2025

Thanks for filing the issue, I had a quick look, I'm pretty sure there is something goofy going on in semantic analysis where the x[0] alone is evaluated in hopes for an l-value, but the error message from attempting []= is ignored.

This might be a precedence issue though, which could use a bit of design discussion, there are two particularly relevant interpretations of x[0] = 1:

  1. x[0] is an l-value producing overload and x[0] = 1 assigns 1 to it
  2. x[0] = 1 is interpreted to mean []=(x, 0, 1)

If we consider the first to be of higher precedence then at most the fix should do is provide a hint, that perhaps you haven't defined/imported an []= overload. Whereas if the second to be higher precedence then the error message as suggested in the issue is correct and the matching rules in the compiler need further revision.

update: I've revised my thinking and agree with @zerbina, see their comments below, as to why the second approach should be higher precedence.

Presently my thinking is, since the lhs, x[0], is an independent sub expression for consideration we should consider that first independently, and therefore the first interpretation (l-value assignment) should take precedence and we provide an additional hint.

@zerbina
Copy link
Collaborator

zerbina commented Jan 19, 2025

For more context, here's an earlier report of the same issue, albeit reported specifically in the context of Table: #1439.

To recapitulate what the compiler does at present (using a[b] = c as an example):

  1. check whether a[b] uses some built-in bracket operator (semSubscript)
  2. if that fails (i.e., when semSubscript returns an nkCall tree), []=(a, b, c) is tried
  3. if there's no user-defined []= for the operands, the system-defined []= (which corresponds to the mArrPut magic) matches
  4. the mArrPut magic call is eagerly turned back into an assignment, which is then analyzed again, but now with the []= fallback disabled
  5. since x[0] is not a built-in bracket operation, semAsgn (with the []= fallback disabled), produces a not-found error for [], which gathers all [] routines in scope, even though none were tried

Formulated in a less implementation-centric way: x[0] is tried as a built-in bracket operator first, and if that bracket is not one of the built-in index operators, the []=(a, b, c) interpretation is tried. If that fails too, the code is illegal; user-defined [] operators are not considered.

@zerbina
Copy link
Collaborator

zerbina commented Jan 20, 2025

The more I think about, the more I think that []=(a, b, c) should take precedence, with my reasoning being:

  • []=(a, b, c) is more specific than [](a, b) = c
  • more specific interpretations should take precedence over less specific ones

This would eliminate a practical problem with interpreting a[b] = c as "assign c to l-value expression a[b]" first. Consider the following:

type Container = object
  # ...

proc `[]`(a: var Container, i: int): var int =
  # ...

proc `[]=`(a: var Container, i: int, val: int) =
  # ...

var c = Container()
c[0] = 1

c[0] = 1 would resolve to [](c, 0) = 1, even though []=(c, 0, 1) is what's preferred and/or expected here, with no way to opt-out (except for hiding the [] via macro or import shenanigans). A real-world API affected by such change would be the Table API.

Giving precedence to []=(c, 0, 1) does provide a syntactic way to opt-out, namely (c[0]) = 1. Of course, that still leaves open what to do when the LHS of an assignment is a template/macro invocation resolving to a bracket expression. Intuitively, I'd say that templ() = x should be treated as (<template-result-goes-here>) = x (meaning that []= operators would not be considered), but I'm not entirely sure whether that's a good idea.

@saem
Copy link
Collaborator

saem commented Jan 21, 2025

Of course, that still leaves open what to do when the LHS of an assignment is a template/macro invocation resolving to a bracket expression. Intuitively, I'd say that templ() = x should be treated as (<template-result-goes-here>) = x (meaning that []= operators would not be considered), but I'm not entirely sure whether that's a good idea.

That's tricky, because the []= precedence decision is made syntactically, and not really semantically (even though it's done in sem). Upon evaluating templ() in templ() = x, I think if we rechecked the newly inserted LHS for []= matching we'd in some ways be violating template hygiene, as the emitted fragment should be considered a logical whole.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler good first issue Easy-to-start-with issue with no in-depth knowledge or complex implementation required.
Projects
None yet
Development

No branches or pull requests

3 participants