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.

Sign up for GitHub

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

Jump to bottom

Unittest breaks if proc name on/off defined in same module #22841

Closed
ghost opened this issue Oct 18, 2023 · 3 comments · Fixed by #23123
Closed

Unittest breaks if proc name on/off defined in same module #22841

ghost opened this issue Oct 18, 2023 · 3 comments · Fixed by #23123

Comments

@ghost
Copy link

ghost commented Oct 18, 2023

Description

There is an error if a proc named on or off is defined in the same module as unittest is used.

import unittest

proc on() =
    discard

suite "some suite":
    test "some test":
        discard

Nim Version

Nim Compiler Version 2.1.1 [Windows: amd64]

0d4b3ed

This occurs in 2.0.0 as well.

Current Output

test_on.nim(6, 1) template/generic instantiation of `suite` from here
nim_tui\test_on.nim(7, 5) template/generic instantiation of `test` from here 
nim-#devel\lib\pure\unittest.nim(555, 34) Error: type mismatch: got 'None' for 'on' but expected 'bool'

Expected Output

No response

Possible Solution

No response

Additional Information

No response
@ghost ghost changed the title Think about the title, twice. Unittest breaks if proc name on/off defined in same module Oct 18, 2023
@metagn
Copy link
Collaborator

metagn commented Oct 18, 2023

Caused by test template being dirty, we could add on and off to the bind list but that would be kind of ridiculous, in general maybe the compiler should just special case on and off in pragmas and not rely on the system.on and system.off constants

@juancarlospaco
Copy link
Collaborator

Different solution, have some kind of strictBool that disables on and off alias?. 🤔

@beef331
Copy link
Collaborator

beef331 commented Oct 19, 2023

Different solution, have some kind of strictBool that disables on and off alias?. 🤔

Soon there will be more Nim dialects than spoken dialects.

@metagn metagn mentioned this issue Dec 1, 2023
Closed
metagn added a commit to metagn/Nim that referenced this issue Dec 18, 2023
@metagn
directly parse on/off in pragmas, ignore user override
53a88b8 
fixes nim-lang#22841, fixes nim-lang#23002
@metagn metagn mentioned this issue Dec 18, 2023
Closed
metagn added a commit to metagn/Nim that referenced this issue Dec 23, 2023
@metagn
ambiguous identifier resolution
f93c93a 
fixes nim-lang#23002, fixes nim-lang#22841
@metagn metagn mentioned this issue Dec 23, 2023
Merged
metagn added a commit to metagn/Nim that referenced this issue Dec 23, 2023
@metagn
solution 3 for ambiguous on/off: dedicated enum
2e2dbf1 
fixes nim-lang#22841, fixes nim-lang#23002, refs nim-lang#23097
@metagn metagn mentioned this issue Dec 23, 2023
Closed
Araq pushed a commit that referenced this issue Jan 1, 2024
@metagn
ambiguous identifier resolution (#23123)
b280100 
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
@metagn @narimiran
ambiguous identifier resolution (#23123)
d723dee 
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
@metagn @narimiran
ambiguous identifier resolution (#23123)
c18e599 
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
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@juancarlospaco @metagn @beef331