Skip to content

Commit

Permalink
resolve unambiguous enum symchoices from local scope, error on rest (#…
Browse files Browse the repository at this point in the history
…22606)

fixes #22598, properly fixes #21887 and fixes test case issue number

When an enum field sym choice has to choose a type, check if its name is
ambiguous in the local scope, then check if the first symbol found in
the local scope is the first symbol in the sym choice. If so, choose
that symbol. Otherwise, give an ambiguous identifier error.

The dependence on the local scope implies this will always give
ambiguity errors for unpicked enum symchoices from generics and
templates and macros from other scopes. We can change `not
isAmbiguous(...) and foundSym == first` to `not (isAmbiguous(...) and
foundSym == first)` to make it so they never give ambiguity errors, and
always pick the first symbol in the symchoice. I can do this if this is
preferred, but no code from CI seems affected.
  • Loading branch information
metagn committed Sep 3, 2023
1 parent d2f36c0 commit 480e98c
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 14 deletions.
35 changes: 35 additions & 0 deletions compiler/lookups.nim
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,41 @@ proc searchInScopesFilterBy*(c: PContext, s: PIdent, filter: TSymKinds): seq[PSy
if s.kind in filter:
result.add s

proc isAmbiguous*(c: PContext, s: PIdent, filter: TSymKinds, sym: var PSym): bool =
result = false
block outer:
for scope in allScopes(c.currentScope):
var ti: TIdentIter
var candidate = initIdentIter(ti, scope.symbols, s)
var scopeHasCandidate = false
while candidate != nil:
if candidate.kind in filter:
if scopeHasCandidate:
# 2 candidates in same scope, ambiguous
return true
else:
scopeHasCandidate = true
sym = candidate
candidate = nextIdentIter(ti, scope.symbols)
if scopeHasCandidate:
# scope had a candidate but wasn't ambiguous
return false

var importsHaveCandidate = false
var marked = initIntSet()
for im in c.imports.mitems:
for s in symbols(im, marked, s, c.graph):
if s.kind in filter:
if importsHaveCandidate:
# 2 candidates among imports, ambiguous
return true
else:
importsHaveCandidate = true
sym = s
if importsHaveCandidate:
# imports had a candidate but wasn't ambiguous
return false

proc errorSym*(c: PContext, n: PNode): PSym =
## creates an error symbol to avoid cascading errors (for IDE support)
var m = n
Expand Down
5 changes: 4 additions & 1 deletion compiler/semexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ proc semExprCheck(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType

proc ambiguousSymChoice(c: PContext, orig, n: PNode): PNode =
let first = n[0].sym
if first.kind == skEnumField:
var foundSym: PSym = nil
if first.kind == skEnumField and
not isAmbiguous(c, first.name, {skEnumField}, foundSym) and
foundSym == first:
# choose the first resolved enum field, i.e. the latest in scope
# to mirror behavior before overloadable enums
if hintAmbiguousEnum in c.config.notes:
Expand Down
26 changes: 26 additions & 0 deletions tests/enum/tambiguousoverloads.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
discard """
cmd: "nim check --hints:off $file"
"""

block: # bug #21887
type
EnumA = enum A = 300, B
EnumB = enum A = 10
EnumC = enum C

doAssert typeof(EnumC(A)) is EnumC #[tt.Error
^ ambiguous identifier 'A' -- use one of the following:
EnumA.A: EnumA
EnumB.A: EnumB]#

block: # issue #22598
type
A = enum
red
B = enum
red

let a = red #[tt.Error
^ ambiguous identifier 'red' -- use one of the following:
A.red: A
B.red: B]#
8 changes: 0 additions & 8 deletions tests/enum/toverloadable_enums.nim
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,3 @@ block: # test with macros/templates
doAssert isOneMS(e2)
doAssert isOneT(e1)
doAssert isOneT(e2)

block: # bug #21908
type
EnumA = enum A = 300, B
EnumB = enum A = 10
EnumC = enum C

doAssert typeof(EnumC(A)) is EnumC
7 changes: 2 additions & 5 deletions tests/lookups/tambsym3.nim
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
discard """
errormsg: "ambiguous enum field"
errormsg: "ambiguous identifier 'mDec' -- use one of the following:"
file: "tambsym3.nim"
line: 14
line: 11
"""
# Test ambiguous symbols

import mambsym1, times

{.hint[AmbiguousEnum]: on.}
{.hintAsError[AmbiguousEnum]: on.}

var
v = mDec #ERROR_MSG ambiguous identifier

Expand Down

0 comments on commit 480e98c

Please sign in to comment.