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

Ambiguous identifier in pragma #23002

Closed
arnetheduck opened this issue Nov 29, 2023 · 6 comments · Fixed by #23123
Closed

Ambiguous identifier in pragma #23002

arnetheduck opened this issue Nov 29, 2023 · 6 comments · Fixed by #23123

Comments

@arnetheduck
Copy link
Contributor

Description

testit2.nim:

proc on*(): void = discard

testit.nim

import testit2

proc test() =
  {.warning[ProveInit]: on.}

test()

inside the pragma, the evaluation should be looking for a bool, not a proc - yet, the proc is matched (which in the real world example has more arguments)

Nim Version

1.6.16, devel

Current Output

testit.nim(4, 25) Error: ambiguous identifier: 'on' -- use one of the following:
  system.on: bool
  testit2.on: proc (){.noSideEffect, gcsafe, locks: 0.}

Expected Output

No response

Possible Solution

No response

Additional Information

No response

@ringabout
Copy link
Member

!nim c

proc on(): void = discard

proc test() =
  {.warning[ProveInit]:on.}

test()

Copy link
Contributor

🐧 Linux bisect by @ringabout (member)
devel 👎 FAIL

Output

Error: Command failed: nim c --run  -d:nimDebug -d:nimDebugDlOpen -d:ssl -d:nimDisableCertificateValidation --forceBuild:on --colors:off --verbosity:0 --hints:off --lineTrace:off --nimcache:/home/runner/work/Nim/Nim --out:/home/runner/work/Nim/Nim/temp /home/runner/work/Nim/Nim/temp.nim
/home/runner/work/Nim/Nim/temp.nim(3, 24) Error: type mismatch: got 'None' for 'on' but expected 'bool'
assertions.nim(34)       raiseAssert
Error: unhandled exception: errGenerated [AssertionDefect]

IR

Compiled filesize 0 bytes (0 bytes)

Stats

  • Started 2023-11-29T11:08:55
  • Finished 2023-11-29T11:08:55
  • Duration

AST

nnkStmtList.newTree(
  nnkProcDef.newTree(
    newIdentNode("on"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newIdentNode("void")
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkDiscardStmt.newTree(
        newEmptyNode()
      )
    )
  ),
  nnkProcDef.newTree(
    newIdentNode("test"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newEmptyNode()
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkPragma.newTree(
        nnkExprColonExpr.newTree(
          nnkBracketExpr.newTree(
            newIdentNode("warning"),
            newIdentNode("ProveInit")
          ),
          newIdentNode("on")
        )
      )
    )
  ),
  nnkCall.newTree(
    newIdentNode("test")
  )
)
stable 👎 FAIL

Output

Error: Command failed: nim c --run  -d:nimDebug -d:nimDebugDlOpen -d:ssl -d:nimDisableCertificateValidation --forceBuild:on --colors:off --verbosity:0 --hints:off --lineTrace:off --nimcache:/home/runner/work/Nim/Nim --out:/home/runner/work/Nim/Nim/temp /home/runner/work/Nim/Nim/temp.nim
/home/runner/work/Nim/Nim/temp.nim(3, 24) Error: type mismatch: got 'None' for 'on' but expected 'bool'
assertions.nim(34)       raiseAssert
Error: unhandled exception: options.nim(664, 5) `false` errGenerated [AssertionDefect]

IR

Compiled filesize 0 bytes (0 bytes)

Stats

  • Started 2023-11-29T11:08:55
  • Finished 2023-11-29T11:08:56
  • Duration

AST

nnkStmtList.newTree(
  nnkProcDef.newTree(
    newIdentNode("on"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newIdentNode("void")
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkDiscardStmt.newTree(
        newEmptyNode()
      )
    )
  ),
  nnkProcDef.newTree(
    newIdentNode("test"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newEmptyNode()
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkPragma.newTree(
        nnkExprColonExpr.newTree(
          nnkBracketExpr.newTree(
            newIdentNode("warning"),
            newIdentNode("ProveInit")
          ),
          newIdentNode("on")
        )
      )
    )
  ),
  nnkCall.newTree(
    newIdentNode("test")
  )
)
2.0.0 👎 FAIL

Output

Error: Command failed: nim c --run  -d:nimDebug -d:nimDebugDlOpen -d:ssl -d:nimDisableCertificateValidation --forceBuild:on --colors:off --verbosity:0 --hints:off --lineTrace:off --nimcache:/home/runner/work/Nim/Nim --out:/home/runner/work/Nim/Nim/temp /home/runner/work/Nim/Nim/temp.nim
/home/runner/work/Nim/Nim/temp.nim(3, 24) Error: type mismatch: got 'None' for 'on' but expected 'bool'
assertions.nim(34)       raiseAssert
Error: unhandled exception: options.nim(664, 5) `false` errGenerated [AssertionDefect]

IR

Compiled filesize 0 bytes (0 bytes)

Stats

  • Started 2023-11-29T11:08:56
  • Finished 2023-11-29T11:08:56
  • Duration

AST

nnkStmtList.newTree(
  nnkProcDef.newTree(
    newIdentNode("on"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newIdentNode("void")
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkDiscardStmt.newTree(
        newEmptyNode()
      )
    )
  ),
  nnkProcDef.newTree(
    newIdentNode("test"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newEmptyNode()
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkPragma.newTree(
        nnkExprColonExpr.newTree(
          nnkBracketExpr.newTree(
            newIdentNode("warning"),
            newIdentNode("ProveInit")
          ),
          newIdentNode("on")
        )
      )
    )
  ),
  nnkCall.newTree(
    newIdentNode("test")
  )
)
1.6.14 👎 FAIL

Output

Error: Command failed: nim c --run  -d:nimDebug -d:nimDebugDlOpen -d:ssl -d:nimDisableCertificateValidation --forceBuild:on --colors:off --verbosity:0 --hints:off --lineTrace:off --nimcache:/home/runner/work/Nim/Nim --out:/home/runner/work/Nim/Nim/temp /home/runner/work/Nim/Nim/temp.nim
/home/runner/work/Nim/Nim/temp.nim(3, 24) Error: type mismatch: got 'None' for 'on' but expected 'bool'
fatal.nim(54)            sysFatal
Error: unhandled exception: options.nim(645, 14) `false` errGenerated [AssertionDefect]

IR

Compiled filesize 0 bytes (0 bytes)

Stats

  • Started 2023-11-29T11:09:00
  • Finished 2023-11-29T11:09:00
  • Duration

AST

nnkStmtList.newTree(
  nnkProcDef.newTree(
    newIdentNode("on"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newIdentNode("void")
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkDiscardStmt.newTree(
        newEmptyNode()
      )
    )
  ),
  nnkProcDef.newTree(
    newIdentNode("test"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newEmptyNode()
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkPragma.newTree(
        nnkExprColonExpr.newTree(
          nnkBracketExpr.newTree(
            newIdentNode("warning"),
            newIdentNode("ProveInit")
          ),
          newIdentNode("on")
        )
      )
    )
  ),
  nnkCall.newTree(
    newIdentNode("test")
  )
)
1.4.8 👎 FAIL

Output

Error: Command failed: nim c --run  -d:nimDebug -d:nimDebugDlOpen -d:ssl -d:nimDisableCertificateValidation --forceBuild:on --colors:off --verbosity:0 --hints:off --lineTrace:off --nimcache:/home/runner/work/Nim/Nim --out:/home/runner/work/Nim/Nim/temp /home/runner/work/Nim/Nim/temp.nim
/home/runner/work/Nim/Nim/temp.nim(3, 24) Error: cannot generate VM code for on

IR

Compiled filesize 0 bytes (0 bytes)

Stats

  • Started 2023-11-29T11:09:02
  • Finished 2023-11-29T11:09:03
  • Duration

AST

nnkStmtList.newTree(
  nnkProcDef.newTree(
    newIdentNode("on"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newIdentNode("void")
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkDiscardStmt.newTree(
        newEmptyNode()
      )
    )
  ),
  nnkProcDef.newTree(
    newIdentNode("test"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newEmptyNode()
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkPragma.newTree(
        nnkExprColonExpr.newTree(
          nnkBracketExpr.newTree(
            newIdentNode("warning"),
            newIdentNode("ProveInit")
          ),
          newIdentNode("on")
        )
      )
    )
  ),
  nnkCall.newTree(
    newIdentNode("test")
  )
)
1.2.18 👎 FAIL

Output

Error: Command failed: nim c --run  -d:nimDebug -d:nimDebugDlOpen -d:ssl -d:nimDisableCertificateValidation --forceBuild:on --colors:off --verbosity:0 --hints:off --lineTrace:off --nimcache:/home/runner/work/Nim/Nim --out:/home/runner/work/Nim/Nim/temp /home/runner/work/Nim/Nim/temp.nim
/home/runner/work/Nim/Nim/temp.nim(3, 24) Error: type mismatch: got <None> but expected 'bool'

IR

Compiled filesize 0 bytes (0 bytes)

Stats

  • Started 2023-11-29T11:09:06
  • Finished 2023-11-29T11:09:06
  • Duration

AST

nnkStmtList.newTree(
  nnkProcDef.newTree(
    newIdentNode("on"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newIdentNode("void")
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkDiscardStmt.newTree(
        newEmptyNode()
      )
    )
  ),
  nnkProcDef.newTree(
    newIdentNode("test"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newEmptyNode()
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkPragma.newTree(
        nnkExprColonExpr.newTree(
          nnkBracketExpr.newTree(
            newIdentNode("warning"),
            newIdentNode("ProveInit")
          ),
          newIdentNode("on")
        )
      )
    )
  ),
  nnkCall.newTree(
    newIdentNode("test")
  )
)
1.0.10 👎 FAIL

Output

Error: Command failed: nim c --run  -d:nimDebug -d:nimDebugDlOpen -d:ssl -d:nimDisableCertificateValidation --forceBuild:on --colors:off --verbosity:0 --hints:off --lineTrace:off --nimcache:/home/runner/work/Nim/Nim --out:/home/runner/work/Nim/Nim/temp /home/runner/work/Nim/Nim/temp.nim
/home/runner/work/Nim/Nim/temp.nim(3, 24) Error: type mismatch: got <None> but expected 'bool'

IR

Compiled filesize 0 bytes (0 bytes)

Stats

  • Started 2023-11-29T11:09:09
  • Finished 2023-11-29T11:09:09
  • Duration

AST

nnkStmtList.newTree(
  nnkProcDef.newTree(
    newIdentNode("on"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newIdentNode("void")
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkDiscardStmt.newTree(
        newEmptyNode()
      )
    )
  ),
  nnkProcDef.newTree(
    newIdentNode("test"),
    newEmptyNode(),
    newEmptyNode(),
    nnkFormalParams.newTree(
      newEmptyNode()
    ),
    newEmptyNode(),
    newEmptyNode(),
    nnkStmtList.newTree(
      nnkPragma.newTree(
        nnkExprColonExpr.newTree(
          nnkBracketExpr.newTree(
            newIdentNode("warning"),
            newIdentNode("ProveInit")
          ),
          newIdentNode("on")
        )
      )
    )
  ),
  nnkCall.newTree(
    newIdentNode("test")
  )
)
Stats
  • GCC 11.4.0
  • Clang 14.0.0
  • NodeJS 18.2
  • Created 2023-11-29T11:08:20Z
  • Comments 1
  • Commands nim c --run -d:nimDebug -d:nimDebugDlOpen -d:ssl -d:nimDisableCertificateValidation --forceBuild:on --colors:off --verbosity:0 --hints:off --lineTrace:off --nimcache:/home/runner/work/Nim/Nim --out:/home/runner/work/Nim/Nim/temp /home/runner/work/Nim/Nim/temp.nim

🤖 Bug found in 18 minutes bisecting 7 commits at 0 commits per second

@ringabout
Copy link
Member

ringabout commented Nov 29, 2023

proc on = discard
const s: bool = on

I image it to be similar with this case, viz, values are not be overloadable except enums etc.

@metagn
Copy link
Collaborator

metagn commented Dec 1, 2023

In #22841 I suggested special casing on/off for pragmas.

We would not have a problem like this if we used true and false instead of on and off, because we know the expected type is a bool which is an enum containing true and false, but on and off are just random constants in system. There also wouldn't be a problem if on and off were defined as type PragmaOption = enum on, off and this was the expected type instead of bool.

Instead of doing this though we can just interpret on and off at the AST level for pragmas and not let the user override their meaning.

@arnetheduck
Copy link
Contributor Author

but on and off are just random constants in system

they're random constants with a type though and since the pragma expects "something with the bool type", it should reasonably be possible to narrow down the matching options.

hardcoding seems like a loss - I'd rather see that "anything that evaluates to a bool" gets done, and then we can even have computed pragma values, ie full expressions.

@metagn
Copy link
Collaborator

metagn commented Dec 1, 2023

We would still be able to compute arbitrary expressions, on and off would just get an early pass (the logic is here).

The type inference logic is there for overloaded symbols (so unfortunately not const), but even with type inference the problem remains if the other module defines another const on: bool. Unlikely, but still interfering with what is always expected to work in the language. Type sections have something similar.

metagn added a commit to metagn/Nim that referenced this issue Dec 18, 2023
metagn added a commit to metagn/Nim that referenced this issue Dec 23, 2023
metagn added a commit to metagn/Nim that referenced this issue Dec 23, 2023
Araq pushed a commit that referenced this issue Jan 1, 2024
fixes #23002, fixes #22841, refs comments in #23097

When an identifier is ambiguous in scope (i.e. multiple imports contain
symbols with the same name), attempt resolving it through type inference
(by creating a symchoice). To do this efficiently, `qualifiedLookUp` had
to be broken up so that `semExpr` can access the ambiguous candidates
directly (now obtained directly via `lookUpCandidates`).

This fixes the linked issues, but an example like:

```nim
let on = 123

{.warning[ProveInit]: on.}
```

will still fail, since `on` is unambiguously the local `let` symbol here
(this is also true for `proc on` but `proc` symbols generate symchoices
anyway).

Type symbols are not considered to not confuse the type inference. This
includes the change in sigmatch, up to this point symchoices with
nonoverloadable symbols could be created, they just wouldn't be
considered during disambiguation. Now every proper symbol except types
are considered in disambiguation, so the correct symbols must be picked
during the creation of the symchoice node. I remember there being a
violating case of this in the compiler, but this was very likely fixed
by excluding type symbols as CI seems to have found no issues.

The pure enum ambiguity test was disabled because ambiguous pure enums
now behave like overloadable enums with this behavior, so we get a
longer error message for `echo amb` like `type mismatch: got <MyEnum |
OtherEnum> but expected T`
narimiran pushed a commit that referenced this issue Aug 29, 2024
fixes #23002, fixes #22841, refs comments in #23097

When an identifier is ambiguous in scope (i.e. multiple imports contain
symbols with the same name), attempt resolving it through type inference
(by creating a symchoice). To do this efficiently, `qualifiedLookUp` had
to be broken up so that `semExpr` can access the ambiguous candidates
directly (now obtained directly via `lookUpCandidates`).

This fixes the linked issues, but an example like:

```nim
let on = 123

{.warning[ProveInit]: on.}
```

will still fail, since `on` is unambiguously the local `let` symbol here
(this is also true for `proc on` but `proc` symbols generate symchoices
anyway).

Type symbols are not considered to not confuse the type inference. This
includes the change in sigmatch, up to this point symchoices with
nonoverloadable symbols could be created, they just wouldn't be
considered during disambiguation. Now every proper symbol except types
are considered in disambiguation, so the correct symbols must be picked
during the creation of the symchoice node. I remember there being a
violating case of this in the compiler, but this was very likely fixed
by excluding type symbols as CI seems to have found no issues.

The pure enum ambiguity test was disabled because ambiguous pure enums
now behave like overloadable enums with this behavior, so we get a
longer error message for `echo amb` like `type mismatch: got <MyEnum |
OtherEnum> but expected T`

(cherry picked from commit b280100)
narimiran pushed a commit that referenced this issue Aug 31, 2024
fixes #23002, fixes #22841, refs comments in #23097

When an identifier is ambiguous in scope (i.e. multiple imports contain
symbols with the same name), attempt resolving it through type inference
(by creating a symchoice). To do this efficiently, `qualifiedLookUp` had
to be broken up so that `semExpr` can access the ambiguous candidates
directly (now obtained directly via `lookUpCandidates`).

This fixes the linked issues, but an example like:

```nim
let on = 123

{.warning[ProveInit]: on.}
```

will still fail, since `on` is unambiguously the local `let` symbol here
(this is also true for `proc on` but `proc` symbols generate symchoices
anyway).

Type symbols are not considered to not confuse the type inference. This
includes the change in sigmatch, up to this point symchoices with
nonoverloadable symbols could be created, they just wouldn't be
considered during disambiguation. Now every proper symbol except types
are considered in disambiguation, so the correct symbols must be picked
during the creation of the symchoice node. I remember there being a
violating case of this in the compiler, but this was very likely fixed
by excluding type symbols as CI seems to have found no issues.

The pure enum ambiguity test was disabled because ambiguous pure enums
now behave like overloadable enums with this behavior, so we get a
longer error message for `echo amb` like `type mismatch: got <MyEnum |
OtherEnum> but expected T`

(cherry picked from commit b280100)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants