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

Stricter checks for typedesc #8657

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions compiler/semstmts.nim
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,9 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode =
var typ: PType
if a.sons[length-2].kind != nkEmpty:
typ = semTypeNode(c, a.sons[length-2], nil)
if typ.kind == tyTypeDesc and c.p.owner.kind != skMacro:
localError(c.config, a.sons[length-2].info, "'typedesc' metatype is not valid here")
continue
else:
typ = nil
var def: PNode = c.graph.emptyNode
Expand Down Expand Up @@ -539,6 +542,9 @@ proc semConst(c: PContext, n: PNode): PNode =
if typ == nil:
localError(c.config, a.sons[2].info, errConstExprExpected)
continue
if typ.kind == tyTypeDesc:
localError(c.config, a.info, "cannot have typedesc as const value, " &
"use 'type $1 = $2' instead", [$a.sons[0], typeToString(def.typ.skipTypes({tyTypeDesc}))])
if typeAllowed(typ, skConst) != nil and def.kind != nkNilLit:
localError(c.config, a.info, "invalid type for const: " & typeToString(typ))
continue
Expand Down
4 changes: 2 additions & 2 deletions compiler/types.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1188,8 +1188,8 @@ proc typeAllowedAux(marker: var IntSet, typ: PType, kind: TSymKind,
if result.isNil and t.sons[0] != nil:
result = typeAllowedAux(marker, t.sons[0], skResult, flags)
of tyTypeDesc:
# XXX: This is still a horrible idea...
result = nil
# Could be inside a macro, so var and let are allowed here and checked later
if kind notin {skParam, skResult, skVar, skLet}: result = t
of tyExpr, tyStmt, tyStatic:
if kind notin {skParam, skResult}: result = t
of tyVoid:
Expand Down
8 changes: 8 additions & 0 deletions tests/errmsgs/t8610.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
discard """
errormsg: "cannot have typedesc as const value, use 'type Foo = int' instead"
line: 6
"""

const Foo=int
echo Foo is int # true
echo int is Foo # Error: type expected
8 changes: 8 additions & 0 deletions tests/errmsgs/t8654_1.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
discard """
errormsg: "'typedesc' metatype is not valid here"
line: 6
"""

var a: typedesc
a = typedesc[int]
echo a is typedesc[int]
7 changes: 7 additions & 0 deletions tests/errmsgs/t8654_2.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
discard """
errormsg: "cannot have typedesc as const value, use 'type a = int' instead"
line: 6
"""

const a: typedesc = typedesc[int]
echo a is typedesc[int]
Copy link
Member

Choose a reason for hiding this comment

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

if you're gonna error on line 6, why have line 7? (+ same in other files)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sanity check? I copied existing tests

Copy link
Member

Choose a reason for hiding this comment

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

why is that a sanity check, if it won't even get compiled?
seems like you could put whatever there instead...
maybe existing tests could be updated too