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

generate symchoice for ambiguous types in templates & generics + handle types in symchoices #23997

Merged
merged 10 commits into from
Aug 25, 2024
7 changes: 7 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:"
Expand Down
14 changes: 9 additions & 5 deletions compiler/semexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
41 changes: 26 additions & 15 deletions compiler/semgnrc.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 =
Expand All @@ -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
Expand All @@ -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) =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
16 changes: 11 additions & 5 deletions compiler/semtempl.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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])
Expand Down
8 changes: 7 additions & 1 deletion compiler/sigmatch.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/pure/ioselects/ioselectors_select.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Araq marked this conversation as resolved.
Show resolved Hide resolved
else:
tv.tv_sec = timeout.int32 div 1_000
tv.tv_usec = (timeout.int32 %% 1_000) * 1_000
Expand Down
File renamed without changes.
4 changes: 4 additions & 0 deletions tests/lookups/mambtype2.nim
Original file line number Diff line number Diff line change
@@ -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]
4 changes: 0 additions & 4 deletions tests/lookups/mqualifiedamb2.nim

This file was deleted.

20 changes: 20 additions & 0 deletions tests/lookups/tambtype.nim
Original file line number Diff line number Diff line change
@@ -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]()
4 changes: 0 additions & 4 deletions tests/lookups/tqualifiedamb.nim

This file was deleted.