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

sem: all tuple fields now have an owning sym #816

Draft
wants to merge 5 commits into
base: devel
Choose a base branch
from
Draft

sem: all tuple fields now have an owning sym #816

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ebb12a7
formatting
saem Jul 26, 2023
5e104da
For tuples without names create a anonymous sym as
saem Jul 26, 2023
bd085a0
refactor(sem): move `newAnonSym` to `sem`
saem Jul 27, 2023
f9cb940
formatting
saem Jul 27, 2023
4b280ff
fix(sem): missing discriminant too large error
saem Jul 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions compiler/sem/semstmts.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1789,7 +1789,6 @@ proc typeDefLeftSidePass(c: PContext, typeSection: PNode, i: int) =
if pkg.isNil or pkg.kind != skPackage:
localReport(c.config, name.info, reportStr(
rsemUnknownPackageName, pkgName.s))

else:
let typsym = c.graph.packageTypes.strTableGet(typName)
if typsym.isNil:
Expand All @@ -1805,7 +1804,6 @@ proc typeDefLeftSidePass(c: PContext, typeSection: PNode, i: int) =
else:
localReport(c.config, name.info, reportSym(
rsemTypeCannotBeForwarded, typsym))

s = typsym
else:
s = semIdentDef(c, name, skType)
Expand All @@ -1821,7 +1819,6 @@ proc typeDefLeftSidePass(c: PContext, typeSection: PNode, i: int) =
# check if we got any errors and if so report them
for e in ifErrorWalkErrors(c.config, name[1]):
localReport(c.config, e)

if sfForward in s.flags:
# check if the symbol already exists:
let pkg = c.module.owner
Expand All @@ -1836,12 +1833,10 @@ proc typeDefLeftSidePass(c: PContext, typeSection: PNode, i: int) =
else:
localReport(c.config, name.info, reportSymbols(
rsemDoubleCompletionOf, @[typsym, s]))

s = typsym
# add it here, so that recursive types are possible:
if sfGenSym notin s.flags:
addInterfaceDecl(c, s)

elif s.owner == nil:
s.owner = getCurrOwner(c)

Expand All @@ -1863,7 +1858,6 @@ proc typeSectionLeftSidePass(c: PContext, n: PNode) =
if a.kind == nkCommentStmt: continue
if a.kind != nkTypeDef:
semReportIllformedAst(c.config, a, {nkTypeDef})

typeDefLeftSidePass(c, n, i)

proc checkCovariantParamsUsages(c: PContext; genericType: PType) =
Expand Down Expand Up @@ -1940,14 +1934,12 @@ proc typeSectionRightSidePass(c: PContext, n: PNode) =
if a.kind == nkCommentStmt: continue
if a.kind != nkTypeDef:
semReportIllformedAst(c.config, a, {nkTypeDef})

checkSonsLen(a, 3, c.config)
let name = typeSectionTypeName(c, a[0])
var s = name.sym
if s.magic == mNone and a[2].kind == nkEmpty:
localReport(c.config, a.info, reportSym(
rsemImplementationExpected, s))

if s.magic != mNone:
processMagicType(c, s)

Expand All @@ -1970,6 +1962,8 @@ proc typeSectionRightSidePass(c: PContext, n: PNode) =
# we fill it out later. For magic generics like 'seq', it won't be filled
# so we use tyNone instead of nil to not crash for strange conversions
# like: mydata.seq
# xxx: this proxy type and passing `nil` to `semTypeNode`'s previous type
# all seem like bad ideas
rawAddSon(s.typ, newTypeS(tyNone, c))
s.ast = a
inc c.inGenericContext
Expand Down Expand Up @@ -1997,12 +1991,10 @@ proc typeSectionRightSidePass(c: PContext, n: PNode) =
# process the type's body:
pushOwner(c, s)
var t = semTypeNode(c, a[2], s.typ)

if t.kind == tyError and t.n.isError:
# we've got a tyError with a report in n
# xxx: we should probably propagate tyError like nkError
c.config.localReport(t.n)

if s.typ == nil:
s.typ = t
elif t != s.typ and (s.typ == nil or s.typ.kind != tyAlias):
Expand All @@ -2015,7 +2007,7 @@ proc typeSectionRightSidePass(c: PContext, n: PNode) =
# final pass
if a[2].kind in nkCallKinds:
incl a[2].flags, nfSem # bug #10548

if sfExportc in s.flags and s.typ.kind == tyAlias:
localReport(c.config, name.info, reportSym(
rsemUnexpectedExportcInAlias, s))
Expand Down
44 changes: 26 additions & 18 deletions compiler/sem/semtypes.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,6 @@ proc semTypeIdent(c: PContext, n: PNode): PSym =
result = qualifiedLookUp(c, n, {checkAmbiguity, checkUndeclared})
if result.isError:
markUsed(c, n.info, result)

# XXX: move to propagating nkError, skError, and tyError
localReport(c.config, result.ast)
elif result != nil:
Expand All @@ -447,7 +446,6 @@ proc semTypeIdent(c: PContext, n: PNode): PSym =
if result.typ.sym == nil:
let err = newError(c.config, n, PAstDiag(kind: adSemTypeExpected))
localReport(c.config, err)

return errorSym(c, n, err)
result = result.typ.sym.copySym(nextSymId c.idgen)
result.typ = exactReplica(result.typ)
Expand Down Expand Up @@ -479,7 +477,6 @@ proc semTypeIdent(c: PContext, n: PNode): PSym =
let err = newError(c.config, n, PAstDiag(kind: adSemTypeExpected))
if result.kind != skError:
localReport(c.config, err)

return errorSym(c, n, err)
if result.typ.kind != tyGenericParam:
# XXX get rid of this hack!
Expand All @@ -500,6 +497,17 @@ proc semAnonTuple(c: PContext, n: PNode, prev: PType): PType =
if n.len == 0:
localReport(c.config, n, reportSem rsemTypeExpected)
result = newOrPrevType(tyTuple, prev, c)
let setupOwner = c.inGenericContext == 0 and result.sym.isNil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't verified it, but I think the check is an issue in the following case:

  type
    Generic[T] = object
      f: tuple[a, b: int]

When the tuple type is sem-checked, inGenericContext is greater than 0 and result.sym is nil.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh, good call.

I was suspicious of the owner pushing in the right hand side type section analysis for generics, and I think that's coming home to roost here.

if setupOwner:
# xxx: instead of "AnonType" maybe generate a name from the field
# names+types with some caching so the same structures have the same
# names/symbols.
let sym = newSym(skType, getIdent(c.cache, "AnonType"),
nextSymId c.idgen, c.getCurrOwner(), n.info)
sym.flags.incl sfAnon
sym.typ = result
result.owner = sym
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a symbol directly represents the type, then it is generally assigned to the types's sym field instead of owner, which for consistency would also make sense here, I think. (typesrenderer already checks for sfAnon symbols, so this should not cause problems)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did, but more so for the silly getTypeDecl and that whole silly API in std/macros. I'll have to dig in further to see what's going wrong there.

pushOwner(c, sym)
for it in n:
let t = semTypeNode(c, it, nil).skipIntLit(c.idgen)
if prev != nil and prev.aliasesType(t):
Expand All @@ -508,13 +516,26 @@ proc semAnonTuple(c: PContext, n: PNode, prev: PType): PType =
break
else:
rawAddSon(result, t)
if setupOwner:
popOwner(c)

proc semTuple(c: PContext, n: PNode, prev: PType): PType =
# TODO: replace with a node returning variant that can in band errors
addInNimDebugUtils(c.config, "semTuple", n, prev, result)
var typ: PType
result = newOrPrevType(tyTuple, prev, c)
result.n = newNodeI(nkRecList, n.info)
let setupOwner = c.inGenericContext == 0 and result.sym.isNil
if setupOwner:
# xxx: instead of "AnonType" maybe generate a name from the field
# names+types with some caching so the same structures have the same
# names/symbols.
let sym = newSym(skType, getIdent(c.cache, "AnonType"),
nextSymId c.idgen, c.getCurrOwner(), n.info)
sym.flags.incl sfAnon
sym.typ = result
result.owner = sym
pushOwner(c, sym)
var check = initIntSet()
var counter = 0
for i in ord(n.kind == nkBracketExpr)..<n.len:
Expand All @@ -523,17 +544,14 @@ proc semTuple(c: PContext, n: PNode, prev: PType): PType =
c.config.globalReport(reportAst(
rsemIllformedAst, a,
str = "Expected identDefs for node, but found " & $a.kind))

checkMinSonsLen(a, 3, c.config)
if a[^2].kind != nkEmpty:
typ = semTypeNode(c, a[^2], nil)
else:
localReport(c.config, a, reportSem rsemTypeExpected)
typ = errorType(c)

if a[^1].kind != nkEmpty:
localReport(c.config, a[^1], reportSem rsemInitHereNotAllowed)

for j in 0 ..< a.len - 2:
let
fieldNode = newSymGNode(skField, a[j], c)
Expand All @@ -547,13 +565,13 @@ proc semTuple(c: PContext, n: PNode, prev: PType): PType =
else:
result.n.add newSymNode(field)
addSonSkipIntLit(result, typ, c.idgen)

styleCheckDef(c.config, a[j].info, field)

if result.n.len == 0: result.n = nil
if isTupleRecursive(result):
localReport(c.config, n.info, reportTyp(
rsemIllegalRecursion, result))
if setupOwner:
popOwner(c)

proc semIdentVis(c: PContext, kind: TSymKind, n: PNode,
allowed: TSymFlags): PSym =
Expand Down Expand Up @@ -862,7 +880,6 @@ proc semRecordNodeAux(c: PContext, n: PNode, check: var IntSet, pos: var int,
var it = n[i]
if it == nil:
semReportIllformedAst(c.config, n, "nil")

var idx = 1
case it.kind
of nkElifBranch:
Expand All @@ -880,7 +897,6 @@ proc semRecordNodeAux(c: PContext, n: PNode, check: var IntSet, pos: var int,
else:
semReportIllformedAst(
c.config, n, "Expected elifBranch of else, but found" & $it.kind)

if c.inGenericContext > 0:
# use a new check intset here for each branch:
var newCheck: IntSet
Expand Down Expand Up @@ -936,12 +952,10 @@ proc semRecordNodeAux(c: PContext, n: PNode, check: var IntSet, pos: var int,
inc(pos)
if containsOrIncl(check, f.name.id):
localReport(c.config, info, reportSym(rsemRedefinitionOf, f))

if a.kind == nkEmpty:
father.add newSymNode(f)
else:
a.add newSymNode(f)

styleCheckDef(c.config, f)
if a.kind != nkEmpty: father.add a
of nkSym:
Expand All @@ -950,7 +964,6 @@ proc semRecordNodeAux(c: PContext, n: PNode, check: var IntSet, pos: var int,
# There is no branch validity check here
if containsOrIncl(check, n.sym.name.id):
localReport(c.config, n.info, reportSym(rsemRedefinitionOf, n.sym))

father.add n
of nkEmpty:
if father.kind in {nkElse, nkOfBranch}:
Expand Down Expand Up @@ -1026,7 +1039,6 @@ proc semObjectNode(c: PContext, n: PNode, prev: PType; flags: TTypeFlags): PType
if concreteBase.kind != tyError:
localReport(c.config, n[1].info, reportTyp(
rsemExpectNonFinalForBase, realBase))

base = nil
realBase = nil
c.config.internalAssert(n.kind == nkObjectTy, n.info, "semObjectNode")
Expand All @@ -1049,7 +1061,6 @@ proc semObjectNode(c: PContext, n: PNode, prev: PType; flags: TTypeFlags): PType
# check if we got any errors and if so report them
for e in ifErrorWalkErrors(c.config, n[0]):
localReport(c.config, e)

if base == nil and tfInheritable notin result.flags:
incl(result.flags, tfFinal)

Expand Down Expand Up @@ -2142,7 +2153,6 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType =
var base = semTypeNode(c, n[1], nil)
if base.kind in {tyVar, tyLent}:
localReport(c.config, n.info, reportTyp(rsemVarVarNotAllowed, prev))

base = base[0]
addSonSkipIntLit(result, base, c.idgen)
of mRef: result = semAnyRef(c, n, tyRef, prev)
Expand Down Expand Up @@ -2177,7 +2187,6 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType =
if s.typ == nil:
if s.kind != skError:
localReport(c.config, n, reportSem rsemTypeExpected)

result = newOrPrevType(tyError, prev, c)
elif s.kind == skParam and s.typ.kind == tyTypeDesc:
c.config.internalAssert s.typ.base.kind != tyNone and prev == nil
Expand Down Expand Up @@ -2216,7 +2225,6 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType =
else:
if s.kind != skError:
localReport(c.config, n.info, reportSym(rsemTypeExpected, s))

result = newOrPrevType(tyError, prev, c)
of nkObjectTy: result = semObjectNode(c, n, prev, {})
of nkTupleTy: result = semTuple(c, n, prev)
Expand Down