From 09dcff71c82147b28059c6470bd921afab9c825e Mon Sep 17 00:00:00 2001 From: metagn Date: Sun, 25 Aug 2024 23:24:20 +0300 Subject: [PATCH] generate symchoice for ambiguous types in templates & generics + handle types in symchoices (#23997) fixes #23898, supersedes #23966 and #23990 Since #20631 ambiguous type symbols in templates are rejected outright, now we generate a symchoice for type nodes if they're ambiguous, a generalization of what was done in #22375. This is done for generics as well. Symchoices also handle type symbols better now, ensuring their type is a `typedesc` type; this probably isn't necessary for everything to work but it makes the logic more robust. Similar to #23989, we have to prepare for the fact that ambiguous type symbols behave differently than normal type symbols and either error normally or relegate to other routine symbols if the symbol is being called. Generating a symchoice emulates this behavior, `semExpr` will find the type symbol first, but since the symchoice has other symbols, it will count as an ambiguous type symbol. I know it seems spammy to carry around an ambiguity flag everywhere, but in the future when we have something like #23104 we could just always generate a symchoice, and the symchoice itself would carry the info of whether the first symbol was ambiguous. But this could harm compiler performance/memory use, it might be better to generate it only when we have to, which in the case of type symbols is only when they're ambiguous. --- changelog.md | 7 ++++ compiler/semexprs.nim | 14 ++++--- compiler/semgnrc.nim | 41 ++++++++++++------- compiler/semtempl.nim | 16 +++++--- compiler/sigmatch.nim | 8 +++- lib/pure/ioselects/ioselectors_select.nim | 2 +- .../{mqualifiedamb1.nim => mambtype1.nim} | 0 tests/lookups/mambtype2.nim | 4 ++ tests/lookups/mqualifiedamb2.nim | 4 -- tests/lookups/tambtype.nim | 20 +++++++++ tests/lookups/tqualifiedamb.nim | 4 -- 11 files changed, 85 insertions(+), 35 deletions(-) rename tests/lookups/{mqualifiedamb1.nim => mambtype1.nim} (100%) create mode 100644 tests/lookups/mambtype2.nim delete mode 100644 tests/lookups/mqualifiedamb2.nim create mode 100644 tests/lookups/tambtype.nim delete mode 100644 tests/lookups/tqualifiedamb.nim diff --git a/changelog.md b/changelog.md index 5056d7b3d87b..d98c34474ea6 100644 --- a/changelog.md +++ b/changelog.md @@ -21,6 +21,13 @@ - `owner` in `std/macros` is deprecated. +- Ambiguous type symbols in generic procs and templates now generate symchoice nodes. + Previously; in templates they would error immediately at the template definition, + and in generic procs a type symbol would arbitrarily be captured, losing the + information of the other symbols. This means that generic code can now give + errors for ambiguous type symbols, and macros operating on generic proc AST + may encounter symchoice nodes instead of the arbitrarily resolved type symbol nodes. + ## Standard library additions and changes [//]: # "Changes:" diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 9293f8497b15..8332d404df87 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -3131,12 +3131,16 @@ proc resolveIdentToSym(c: PContext, n: PNode, resultNode: var PNode, # unambiguous, or we don't care about ambiguity result = candidates[0] else: - # ambiguous symbols have 1 last chance as a symchoice, - # but type symbols cannot participate in symchoices + # ambiguous symbols have 1 last chance as a symchoice var choice = newNodeIT(nkClosedSymChoice, n.info, newTypeS(tyNone, c)) - for c in candidates: - if c.kind notin {skType, skModule, skPackage}: - choice.add newSymNode(c, n.info) + for cand in candidates: + case cand.kind + of skModule, skPackage: + discard + of skType: + choice.add newSymNodeTypeDesc(cand, c.idgen, n.info) + else: + choice.add newSymNode(cand, n.info) if choice.len == 0: # we know candidates.len > 1, we just couldn't put any in a symchoice errorUseQualifier(c, n.info, candidates) diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim index e7f36879f0f6..8efc8a94ecdd 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -64,6 +64,7 @@ proc newOpenSym*(n: PNode): PNode {.inline.} = proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, ctx: var GenericCtx; flags: TSemGenericFlags, + isAmbiguous: bool, fromDotExpr=false): PNode = result = nil semIdeForTemplateOrGenericCheck(c.config, n, ctx.cursorInBody) @@ -133,6 +134,11 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, of skType: if (s.typ != nil) and (s.typ.flags * {tfGenericTypeParam, tfImplicitTypeParam} == {}): + if isAmbiguous: + # ambiguous types should be symchoices since lookup behaves + # differently for them in regular expressions + maybeDotChoice(c, n, s, fromDotExpr) + return result = newSymNodeTypeDesc(s, c.idgen, n.info) if canOpenSym(result.sym): if genericsOpenSym in c.features: @@ -184,7 +190,7 @@ proc lookup(c: PContext, n: PNode, flags: TSemGenericFlags, elif s.isMixedIn: result = symChoice(c, n, s, scForceOpen) else: - result = semGenericStmtSymbol(c, n, s, ctx, flags) + result = semGenericStmtSymbol(c, n, s, ctx, flags, amb) # else: leave as nkIdent proc newDot(n, b: PNode): PNode = @@ -200,10 +206,11 @@ proc fuzzyLookup(c: PContext, n: PNode, flags: TSemGenericFlags, let luf = if withinMixin notin flags: {checkUndeclared, checkModule} else: {checkModule} + c.isAmbiguous = false var s = qualifiedLookUp(c, n, luf) if s != nil: isMacro = s.kind in {skTemplate, skMacro} - result = semGenericStmtSymbol(c, n, s, ctx, flags) + result = semGenericStmtSymbol(c, n, s, ctx, flags, c.isAmbiguous) else: n[0] = semGenericStmt(c, n[0], flags, ctx) result = n @@ -217,24 +224,21 @@ proc fuzzyLookup(c: PContext, n: PNode, flags: TSemGenericFlags, isMacro = s.kind in {skTemplate, skMacro} if withinBind in flags or s.id in ctx.toBind: if s.kind == skType: # don't put types in sym choice - result = newDot(result, semGenericStmtSymbol(c, n, s, ctx, flags, fromDotExpr=true)) + var ambig = false + if candidates.len > 1: + let s2 = searchInScopes(c, ident, ambig) + result = newDot(result, semGenericStmtSymbol(c, n, s, ctx, flags, + isAmbiguous = ambig, fromDotExpr = true)) else: result = newDot(result, symChoice(c, n, s, scClosed)) elif s.isMixedIn: result = newDot(result, symChoice(c, n, s, scForceOpen)) else: + var ambig = false if s.kind == skType and candidates.len > 1: - var ambig = false - let s2 = searchInScopes(c, ident, ambig) - if ambig: - # this is a type conversion like a.T where T is ambiguous with - # other types or routines - # in regular code, this never considers a type conversion and - # skips to routine overloading - # so symchoices are used which behave similarly with type symbols - result = newDot(result, symChoice(c, n, s, scForceOpen)) - return - let syms = semGenericStmtSymbol(c, n, s, ctx, flags, fromDotExpr=true) + discard searchInScopes(c, ident, ambig) + let syms = semGenericStmtSymbol(c, n, s, ctx, flags, + isAmbiguous = ambig, fromDotExpr = true) result = newDot(result, syms) proc addTempDecl(c: PContext; n: PNode; kind: TSymKind) = @@ -301,7 +305,9 @@ proc semGenericStmt(c: PContext, n: PNode, # check if it is an expression macro: checkMinSonsLen(n, 1, c.config) let fn = n[0] + c.isAmbiguous = false var s = qualifiedLookUp(c, fn, {}) + let ambig = c.isAmbiguous if s == nil and {withinMixin, withinConcept}*flags == {} and fn.kind in {nkIdent, nkAccQuoted} and @@ -354,7 +360,12 @@ proc semGenericStmt(c: PContext, n: PNode, of skType: # bad hack for generics: if (s.typ != nil) and (s.typ.kind != tyGenericParam): - result[0] = newSymNodeTypeDesc(s, c.idgen, fn.info) + if ambig: + # ambiguous types should be symchoices since lookup behaves + # differently for them in regular expressions + result[0] = sc + else: + result[0] = newSymNodeTypeDesc(s, c.idgen, fn.info) onUse(fn.info, s) first = 1 else: diff --git a/compiler/semtempl.nim b/compiler/semtempl.nim index c6f2fb60cc62..f7653a890418 100644 --- a/compiler/semtempl.nim +++ b/compiler/semtempl.nim @@ -218,7 +218,7 @@ proc addLocalDecl(c: var TemplCtx, n: var PNode, k: TSymKind) = if k == skParam and c.inTemplateHeader > 0: local.flags.incl sfTemplateParam -proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField: bool): PNode = +proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField, isAmbiguous: bool): PNode = incl(s.flags, sfUsed) # bug #12885; ideally sem'checking is performed again afterwards marking # the symbol as used properly, but the nfSem mechanism currently prevents @@ -245,6 +245,10 @@ proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField: bool): PNode = result = n of skType: if isField and sfGenSym in s.flags: result = n + elif isAmbiguous: + # ambiguous types should be symchoices since lookup behaves + # differently for them in regular expressions + result = symChoice(c, n, s, scOpen, isField) else: result = newSymNodeTypeDesc(s, c.idgen, n.info) else: if isField and sfGenSym in s.flags: result = n @@ -345,6 +349,7 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode = case n.kind of nkIdent: if n.ident.id in c.toInject: return n + c.c.isAmbiguous = false let s = qualifiedLookUp(c.c, n, {}) if s != nil: if s.owner == c.owner and s.kind == skParam and sfTemplateParam in s.flags: @@ -362,9 +367,9 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode = result = newSymNode(s, n.info) onUse(n.info, s) else: - if s.kind in {skType, skVar, skLet, skConst}: + if s.kind in {skVar, skLet, skConst}: discard qualifiedLookUp(c.c, n, {checkAmbiguity, checkModule}) - result = semTemplSymbol(c.c, n, s, c.noGenSym > 0) + result = semTemplSymbol(c.c, n, s, c.noGenSym > 0, c.c.isAmbiguous) of nkBind: result = semTemplBody(c, n[0]) of nkBindStmt: @@ -558,6 +563,7 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode = of nkDotExpr, nkAccQuoted: # dotExpr is ambiguous: note that we explicitly allow 'x.TemplateParam', # so we use the generic code for nkDotExpr too + c.c.isAmbiguous = false let s = qualifiedLookUp(c.c, n, {}) if s != nil: # mirror the nkIdent case @@ -572,9 +578,9 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode = elif contains(c.toMixin, s.name.id): return symChoice(c.c, n, s, scForceOpen, c.noGenSym > 0) else: - if s.kind in {skType, skVar, skLet, skConst}: + if s.kind in {skVar, skLet, skConst}: discard qualifiedLookUp(c.c, n, {checkAmbiguity, checkModule}) - return semTemplSymbol(c.c, n, s, c.noGenSym > 0) + return semTemplSymbol(c.c, n, s, c.noGenSym > 0, c.c.isAmbiguous) if n.kind == nkDotExpr: result = n result[0] = semTemplBody(c, n[0]) diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim index f2865347f788..d0c9ce029d52 100644 --- a/compiler/sigmatch.nim +++ b/compiler/sigmatch.nim @@ -2381,7 +2381,7 @@ proc paramTypesMatch*(m: var TCandidate, f, a: PType, result = paramTypesMatchAux(m, f, a, arg, argOrig) else: # symbol kinds that don't participate in symchoice type disambiguation: - let matchSet = {low(TSymKind)..high(TSymKind)} - {skModule, skPackage, skType} + let matchSet = {low(TSymKind)..high(TSymKind)} - {skModule, skPackage} var best = -1 result = arg @@ -2426,6 +2426,12 @@ proc paramTypesMatch*(m: var TCandidate, f, a: PType, if arg[i].sym.kind in matchSet: copyCandidate(z, m) z.callee = arg[i].typ + if arg[i].sym.kind == skType and z.callee.kind != tyTypeDesc: + # creating the symchoice with the type sym having typedesc type + # breaks a lot of stuff, so we make the typedesc type here + # mirrored from `newSymNodeTypeDesc` + z.callee = newType(tyTypeDesc, c.idgen, arg[i].sym.owner) + z.callee.addSonSkipIntLit(arg[i].sym.typ, c.idgen) if tfUnresolved in z.callee.flags: continue z.calleeSym = arg[i].sym z.calleeScope = cmpScopes(m.c, arg[i].sym) diff --git a/lib/pure/ioselects/ioselectors_select.nim b/lib/pure/ioselects/ioselectors_select.nim index 11bc62b78d6f..6c516395b6cb 100644 --- a/lib/pure/ioselects/ioselectors_select.nim +++ b/lib/pure/ioselects/ioselectors_select.nim @@ -314,7 +314,7 @@ proc selectInto*[T](s: Selector[T], timeout: int, if timeout != -1: when defined(genode) or defined(freertos) or defined(zephyr) or defined(nuttx): - tv.tv_sec = Time(timeout div 1_000) + tv.tv_sec = posix.Time(timeout div 1_000) else: tv.tv_sec = timeout.int32 div 1_000 tv.tv_usec = (timeout.int32 %% 1_000) * 1_000 diff --git a/tests/lookups/mqualifiedamb1.nim b/tests/lookups/mambtype1.nim similarity index 100% rename from tests/lookups/mqualifiedamb1.nim rename to tests/lookups/mambtype1.nim diff --git a/tests/lookups/mambtype2.nim b/tests/lookups/mambtype2.nim new file mode 100644 index 000000000000..cf622466b168 --- /dev/null +++ b/tests/lookups/mambtype2.nim @@ -0,0 +1,4 @@ +import ./mambtype1 +export mambtype1 +template K*(kind: static int): auto = typedesc[mambtype1.K] +template B*(kind: static int): auto = typedesc[mambtype1.K] diff --git a/tests/lookups/mqualifiedamb2.nim b/tests/lookups/mqualifiedamb2.nim deleted file mode 100644 index 3ea5bd04fd6a..000000000000 --- a/tests/lookups/mqualifiedamb2.nim +++ /dev/null @@ -1,4 +0,0 @@ -import ./mqualifiedamb1 -export mqualifiedamb1 -template K*(kind: static int): auto = typedesc[mqualifiedamb1.K] -template B*(kind: static int): auto = typedesc[mqualifiedamb1.K] diff --git a/tests/lookups/tambtype.nim b/tests/lookups/tambtype.nim new file mode 100644 index 000000000000..a292db83ad3c --- /dev/null +++ b/tests/lookups/tambtype.nim @@ -0,0 +1,20 @@ +import ./mambtype2 + +block: # issue #23893 + discard default(K(0)) # works + discard default(mambtype2.B(0)) # works + discard default(mambtype2.K(0)) # doesn't work + +block: # issue #23898, in template + template r() = + discard default(B(0)) # compiles + discard default(mambtype2.B(0)) # compiles + discard default(K(0)) # does not compile + r() + +block: # in generics + proc foo[T]() = + discard default(B(0)) # compiles + discard default(mambtype2.B(0)) # compiles + discard default(K(0)) # does not compile + foo[int]() diff --git a/tests/lookups/tqualifiedamb.nim b/tests/lookups/tqualifiedamb.nim deleted file mode 100644 index a5e1955f3e99..000000000000 --- a/tests/lookups/tqualifiedamb.nim +++ /dev/null @@ -1,4 +0,0 @@ -import ./mqualifiedamb2 -discard default(K(0)) # works -discard default(mqualifiedamb2.B(0)) # works -discard default(mqualifiedamb2.K(0)) # doesn't work