diff --git a/changelog.md b/changelog.md index f4a9d627cf671..9b3e33a5d1804 100644 --- a/changelog.md +++ b/changelog.md @@ -14,6 +14,14 @@ ``` will no longer compile. +- In template and generic bodies, for symbols with only 1 overload in scope, + previously the compiler would not allow other overloads in the instantiation + context to be used, which was not the case for 0 or more than 1 overloads. + Now other overloads are allowed with a preference for the original overload + in cases of ambiguity. Some code might need to be changed to use `bind` to + prevent outside overloads from replacing a weaker captured overload. See + [issue #11184](https://github.com/nim-lang/Nim/issues/11184) for more details. + ## Standard library additions and changes [//]: # "Changes:" diff --git a/compiler/ast.nim b/compiler/ast.nim index 732763f0fe61b..4d3000aadb089 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -521,6 +521,7 @@ type nfHasComment # node has a comment nfSkipFieldChecking # node skips field visable checking nfOpenSym # node is a captured sym but can be overriden by local symbols + nfPreferredSym # node is a preferred sym in a symchoice TNodeFlags* = set[TNodeFlag] TTypeFlag* = enum # keep below 32 for efficiency reasons (now: 47) @@ -1097,7 +1098,7 @@ const nfFromTemplate, nfDefaultRefsParam, nfExecuteOnReload, nfLastRead, nfFirstWrite, nfSkipFieldChecking, - nfOpenSym} + nfOpenSym, nfPreferredSym} namePos* = 0 patternPos* = 1 # empty except for term rewriting macros genericParamsPos* = 2 diff --git a/compiler/lookups.nim b/compiler/lookups.nim index 52296644dd10a..c2aa8d8e42489 100644 --- a/compiler/lookups.nim +++ b/compiler/lookups.nim @@ -268,41 +268,6 @@ proc cmpScopes*(ctx: PContext, s: PSym): int = else: result = 1 -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, ident: PIdent, info: TLineInfo): PSym = ## creates an error symbol to avoid cascading errors (for IDE support) result = newSym(skError, ident, c.idgen, getCurrOwner(c), info, {}) @@ -332,6 +297,7 @@ type m*: PSym mode*: TOverloadIterMode symChoiceIndex*: int + symChoiceLastPreferred: bool currentScope: PScope importIdx: int marked: IntSet @@ -611,6 +577,11 @@ proc lookUp*(c: PContext, n: PNode): PSym = var ident = considerQuotedIdent(c, n) result = searchInScopes(c, ident, amb) if result == nil: result = errorUndeclaredIdentifierHint(c, ident, n.info) + of nkOpenSymChoice, nkClosedSymChoice: + if nfPreferredSym in n[0].flags: + result = n[0].sym + else: + result = nil else: internalError(c.config, n.info, "lookUp") return nil @@ -691,6 +662,11 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym = localError(c.config, n[1].info, "identifier expected, but got: " & renderTree(n[1])) result = errorSym(c, n[1]) + of nkOpenSymChoice, nkClosedSymChoice: + if nfPreferredSym in n[0].flags: + result = n[0].sym + else: + result = nil else: result = nil when false: @@ -723,7 +699,18 @@ proc initOverloadIter*(o: var TOverloadIter, c: PContext, n: PNode): PSym = of nkSym: result = n.sym - o.mode = oimDone + if nfPreferredSym in n.flags: + # standalone sym node with nfPreferredSym acts like an open symchoice, + # see semtempl.symChoice for reasoning + o.mode = oimSymChoiceLocalLookup + o.symChoiceLastPreferred = true + o.currentScope = c.currentScope + o.it.h = result.name.h + o.it.name = result.name + o.marked = initIntSet() + incl(o.marked, result.id) + else: + o.mode = oimDone of nkDotExpr: result = nil o.mode = oimOtherModule @@ -749,6 +736,7 @@ proc initOverloadIter*(o: var TOverloadIter, c: PContext, n: PNode): PSym = o.mode = oimSymChoice if n[0].kind == nkSym: result = n[0].sym + o.symChoiceLastPreferred = nfPreferredSym in n[0].flags else: o.mode = oimDone return nil @@ -767,6 +755,11 @@ proc lastOverloadScope*(o: TOverloadIter): int = else: o.currentScope.depthLevel of oimSelfModule: result = 1 of oimOtherModule: result = 0 + of oimSymChoice, oimSymChoiceLocalLookup: + if o.symChoiceLastPreferred: + result = 999 + else: + result = -1 else: result = -1 proc nextOverloadIterImports(o: var TOverloadIter, c: PContext, n: PNode): PSym = @@ -825,6 +818,7 @@ proc nextOverloadIter*(o: var TOverloadIter, c: PContext, n: PNode): PSym = of oimOtherModule: result = nextModuleIter(o.mit, c.graph) of oimSymChoice: + o.symChoiceLastPreferred = false if o.symChoiceIndex < n.len: result = n[o.symChoiceIndex].sym incl(o.marked, result.id) @@ -849,13 +843,15 @@ proc nextOverloadIter*(o: var TOverloadIter, c: PContext, n: PNode): PSym = else: result = nil of oimSymChoiceLocalLookup: + o.symChoiceLastPreferred = false if o.currentScope != nil: result = nextIdentExcluding(o.it, o.currentScope.symbols, o.marked) while result == nil: o.currentScope = o.currentScope.parent if o.currentScope != nil: + let name = if n.kind == nkSym: n.sym.name else: n[0].sym.name result = firstIdentExcluding(o.it, o.currentScope.symbols, - n[0].sym.name, o.marked) + name, o.marked) else: o.importIdx = 0 result = symChoiceExtension(o, c, n) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 67eee3a19a407..49c2e67b93d36 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -101,7 +101,8 @@ proc semExprWithType(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType elif result.typ.kind == tyError: # associates the type error to the current owner result.typ = errorType(c) - elif efTypeAllowed in flags and result.typ.kind == tyProc and + elif {efTypeAllowed, efOperand} * flags != {} and + result.typ.kind == tyProc and hasUnresolvedParams(result, {}): # mirrored with semOperand but only on efTypeAllowed let owner = result.typ.owner @@ -140,14 +141,9 @@ proc resolveSymChoice(c: PContext, n: var PNode, flags: TExprFlags = {}, expecte if isSymChoice(n) and efAllowSymChoice notin flags: # some contexts might want sym choices preserved for later disambiguation # in general though they are ambiguous - let first = n[0].sym - 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 - n = n[0] + let first = n[0] + if nfPreferredSym in first.flags: + n = first proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode = result = n @@ -1554,8 +1550,7 @@ proc builtinFieldAccess(c: PContext; n: PNode; flags: var TExprFlags): PNode = proc dotTransformation(c: PContext, n: PNode): PNode = if isSymChoice(n[1]) or - # generics usually leave field names as symchoices, but not types - (n[1].kind == nkSym and n[1].sym.kind == skType): + (n[1].kind == nkSym and n[1].sym.kind in routineKinds + {skType}): result = newNodeI(nkDotCall, n.info) result.add n[1] result.add copyTree(n[0]) @@ -2965,13 +2960,17 @@ proc getNilType(c: PContext): PType = c.nilTypeCache = result proc enumFieldSymChoice(c: PContext, n: PNode, s: PSym): PNode = - var o: TOverloadIter + var o: TOverloadIter = default(TOverloadIter) + var firstPreferred = true var i = 0 var a = initOverloadIter(o, c, n) + let firstScope = lastOverloadScope(o) while a != nil: if a.kind == skEnumField: inc(i) - if i > 1: break + if i > 1: + firstPreferred = firstScope > lastOverloadScope(o) + break a = nextOverloadIter(o, c, n) let info = getCallLineInfo(n) if i <= 1: @@ -2991,6 +2990,8 @@ proc enumFieldSymChoice(c: PContext, n: PNode, s: PSym): PNode = result.add newSymNode(a, info) onUse(info, a) a = nextOverloadIter(o, c, n) + if firstPreferred: + result[0].flags.incl nfPreferredSym proc semPragmaStmt(c: PContext; n: PNode) = if c.p.owner.kind == skModule: diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 48a6b9d1c3be0..876bb0758e805 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -1145,7 +1145,7 @@ proc track(tracked: PEffects, n: PNode) = if n.sym.typ != nil and tfHasAsgn in n.sym.typ.flags: tracked.owner.flags.incl sfInjectDestructors # bug #15038: ensure consistency - if not hasDestructor(n.typ) and sameType(n.typ, n.sym.typ): n.typ = n.sym.typ + if n.typ != nil and not hasDestructor(n.typ) and sameType(n.typ, n.sym.typ): n.typ = n.sym.typ of nkHiddenAddr, nkAddr: if n[0].kind == nkSym and isLocalSym(tracked, n[0].sym): useVarNoInitCheck(tracked, n[0], n[0].sym) diff --git a/compiler/semtempl.nim b/compiler/semtempl.nim index e572efdc0bec0..fdfe3161a892a 100644 --- a/compiler/semtempl.nim +++ b/compiler/semtempl.nim @@ -52,23 +52,38 @@ proc symChoice(c: PContext, n: PNode, s: PSym, r: TSymChoiceRule; isField = false): PNode = var a: PSym - o: TOverloadIter + o: TOverloadIter = default(TOverloadIter) + firstPreferred = true var i = 0 a = initOverloadIter(o, c, n) + let firstScope = lastOverloadScope(o) while a != nil: if a.kind != skModule: inc(i) - if i > 1: break + if i > 1: + firstPreferred = firstScope > lastOverloadScope(o) + break a = nextOverloadIter(o, c, n) let info = getCallLineInfo(n) if i <= 1 and r != scForceOpen: - # XXX this makes more sense but breaks bootstrapping for now: - # (s.kind notin routineKinds or s.magic != mNone): - # for instance 'nextTry' is both in tables.nim and astalgo.nim ... if not isField or sfGenSym notin s.flags: result = newSymNode(s, info) - markUsed(c, info, s) - onUse(info, s) + if r == scClosed or n.kind == nkDotExpr or + # also bind magic procs: + (s.magic != mNone and s.kind in routineKinds): + markUsed(c, info, s) + onUse(info, s) + else: + # we need a node with a type here so things like default parameters, + # which use semGenericStmt, can infer the parameter type + # from expressions like `false` + # but symchoices having types can mislead the compiler + # instead we allow standalone sym nodes to have nfPreferredSym + # which acts like an open symchoice in initOverloadIter + result.flags.incl nfPreferredSym + #result = newTreeIT(nkOpenSymChoice, info, result.typ, result) + incl(s.flags, sfUsed) + markOwnerModuleAsUsed(c, s) else: result = n elif i == 0: @@ -88,6 +103,8 @@ proc symChoice(c: PContext, n: PNode, s: PSym, r: TSymChoiceRule; result.add newSymNode(a, info) onUse(info, a) a = nextOverloadIter(o, c, n) + if r != scForceOpen and firstPreferred: + result[0].flags.incl nfPreferredSym proc semBindStmt(c: PContext, n: PNode, toBind: var IntSet): PNode = result = copyNode(n) diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim index 8f383013090e7..9d2a611309d13 100644 --- a/compiler/sigmatch.nim +++ b/compiler/sigmatch.nim @@ -246,7 +246,8 @@ proc sumGeneric(t: PType): int = result += sumGeneric(a) break of tyProc: - result += sumGeneric(t.returnType) + if t.returnType != nil: + result += sumGeneric(t.returnType) for _, a in t.paramTypes: result += sumGeneric(a) break @@ -2416,7 +2417,8 @@ proc paramTypesMatch*(m: var TCandidate, f, a: PType, if x.state != csMatch: internalError(m.c.graph.config, arg.info, "x.state is not csMatch") result = nil - if best > -1 and result != nil: + if (best > -1 and result != nil) or + (best = 0; nfPreferredSym in arg[best].flags): # only one valid interpretation found: markUsed(m.c, arg.info, arg[best].sym) onUse(arg.info, arg[best].sym) diff --git a/lib/pure/collections/hashcommon.nim b/lib/pure/collections/hashcommon.nim index 8fd4c6e086213..70bc04e72044f 100644 --- a/lib/pure/collections/hashcommon.nim +++ b/lib/pure/collections/hashcommon.nim @@ -38,10 +38,10 @@ proc slotsNeeded(count: Natural): int {.inline.} = # Make sure to synchronize with `mustRehash` above result = nextPowerOfTwo(count * 3 div 2 + 4) -template rawGetKnownHCImpl() {.dirty.} = +template rawGetKnownHCImpl() = if t.dataLen == 0: return -1 - var h: Hash = hc and maxHash(t) # start with real hash value + var h {.inject.}: Hash = hc and maxHash(t) # start with real hash value while isFilled(t.data[h].hcode): # Compare hc THEN key with boolean short circuit. This makes the common case # zero ==key's for missing (e.g.inserts) and exactly one ==key for present. diff --git a/tests/enum/tcrossmodule.nim b/tests/enum/tcrossmodule.nim index c21072198ee87..f8b5a876da7c0 100644 --- a/tests/enum/tcrossmodule.nim +++ b/tests/enum/tcrossmodule.nim @@ -9,7 +9,10 @@ template t = t() -block: # legacy support for behavior before overloadableEnums - # warning: ambiguous enum field 'Success' assumed to be of type MyEnum +block: # account for scope let x = {Success} doAssert x is set[MyEnum] + proc foo[T](a: T): string = $a + doAssert foo(Success) == "Success" + proc bar[T](): string = $Success + doAssert bar[int]() == "Success" diff --git a/tests/lookups/mpreferredsym.nim b/tests/lookups/mpreferredsym.nim new file mode 100644 index 0000000000000..9a39a3d85cda5 --- /dev/null +++ b/tests/lookups/mpreferredsym.nim @@ -0,0 +1,28 @@ +# foo0 has 0 overloads + +template myTemplate0*(): string = + foo0(bar) + +# foo1 has 1 overload + +proc foo1(arg: int): string = + "foo1 bad" + +template myTemplate1*(): string = + foo1(bar) + +# foo2 has 2 overloads + +proc foo2(arg: int): string = + "foo2 bad 1" + +proc foo2(arg: string): string = + "foo2 bad 2" + +template myTemplate2*(): string = + foo2(bar) + +proc overloadToPrefer(x: int): int = x + 1 + +template singleOverload*: untyped = + (overloadToPrefer(123), overloadToPrefer("abc")) diff --git a/tests/lookups/tpreferredsym.nim b/tests/lookups/tpreferredsym.nim new file mode 100644 index 0000000000000..ddb5bcc3fce27 --- /dev/null +++ b/tests/lookups/tpreferredsym.nim @@ -0,0 +1,21 @@ +import mpreferredsym + +block: # issue #11184 + type MyType = object + + proc foo0(arg: MyType): string = "foo0" + proc foo1(arg: MyType): string = "foo1" + proc foo2(arg: MyType): string = "foo2" + + proc test() = + var bar: MyType + + doAssert myTemplate0() == "foo0" + doAssert myTemplate1() == "foo1" + doAssert myTemplate2() == "foo2" + + test() + +block: + proc overloadToPrefer(x: string): string = x & "def" + doAssert singleOverload() == (124, "abcdef") diff --git a/tests/template/tbaddeprecated.nim b/tests/template/tbaddeprecated.nim new file mode 100644 index 0000000000000..1d96982c8fad1 --- /dev/null +++ b/tests/template/tbaddeprecated.nim @@ -0,0 +1,13 @@ +# issue #15650 + +{.warningAsError[Deprecated]: on.} + +proc bar() {.deprecated.} = discard + +template foo() = + when false: + bar() + else: + discard + +foo()