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

custom pragmas: correct error condition, remove outdated symkind whitelist #21653

Merged
merged 5 commits into from
Apr 13, 2023
Merged
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
28 changes: 18 additions & 10 deletions compiler/pragmas.nim
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,15 @@ const
proc invalidPragma*(c: PContext; n: PNode) =
localError(c.config, n.info, "invalid pragma: " & renderTree(n, {renderNoComments}))
proc illegalCustomPragma*(c: PContext, n: PNode, s: PSym) =
localError(c.config, n.info, "cannot attach a custom pragma to '" & s.name.s & "'")
var msg = "cannot attach a custom pragma to '" & s.name.s & "'"
if s != nil:
msg.add("; custom pragmas are not supported for ")
case s.kind
of skForVar: msg.add("`for` loop variables")
of skEnumField: msg.add("enum fields")
of skModule: msg.add("modules")
else: msg.add("symbol kind " & $s.kind)
localError(c.config, n.info, msg)

proc pragmaProposition(c: PContext, n: PNode) =
if n.kind notin nkPragmaCallKinds or n.len != 2:
Expand Down Expand Up @@ -755,7 +763,7 @@ proc pragmaGuard(c: PContext; it: PNode; kind: TSymKind): PSym =
else:
result = qualifiedLookUp(c, n, {checkUndeclared})

proc semCustomPragma(c: PContext, n: PNode): PNode =
proc semCustomPragma(c: PContext, n: PNode, sym: PSym): PNode =
var callNode: PNode

if n.kind in {nkIdent, nkSym}:
Expand All @@ -774,6 +782,11 @@ proc semCustomPragma(c: PContext, n: PNode): PNode =
if r.isNil or sfCustomPragma notin r[0].sym.flags:
invalidPragma(c, n)
return n

# we have a valid custom pragma
if sym != nil and sym.kind in {skEnumField, skForVar, skModule}:
illegalCustomPragma(c, n, sym)
return n

result = r
# Transform the nkCall node back to its original form if possible
Expand Down Expand Up @@ -822,7 +835,7 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int,
else: discard
return
elif key.kind notin nkIdentKinds:
n[i] = semCustomPragma(c, it)
n[i] = semCustomPragma(c, it, sym)
return
let ident = considerQuotedIdent(c, key)
var userPragma = strTableGet(c.userPragmas, ident)
Expand Down Expand Up @@ -1248,13 +1261,8 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int,
elif comesFromPush and whichKeyword(ident) != wInvalid:
discard "ignore the .push pragma; it doesn't apply"
else:
if sym == nil or (sym.kind in {skVar, skLet, skConst, skParam, skIterator,
skField, skProc, skFunc, skConverter, skMethod, skType}):
n[i] = semCustomPragma(c, it)
elif sym != nil:
illegalCustomPragma(c, it, sym)
else:
invalidPragma(c, it)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This branch was also never called as sym != nil is guaranteed to be true

So this doesn't change general performance either

# semCustomPragma gives appropriate error for invalid pragmas
n[i] = semCustomPragma(c, it, sym)

proc overwriteLineInfo(n: PNode; info: TLineInfo) =
n.info = info
Expand Down
2 changes: 1 addition & 1 deletion tests/pragmas/t8741.nim
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
discard """
cmd: "nim check --hint:processing:off $file"
errormsg: "3 is not two"
nimout: '''t8741.nim(13, 9) Error: cannot attach a custom pragma to 'a'
nimout: '''t8741.nim(13, 9) Error: invalid pragma: foobar
t8741.nim(29, 15) template/generic instantiation of `onlyTwo` from here
t8741.nim(25, 12) Error: 3 is not two
'''
Expand Down
29 changes: 28 additions & 1 deletion tests/pragmas/tcustom_pragma.nim
Original file line number Diff line number Diff line change
Expand Up @@ -399,12 +399,39 @@ block:

discard Hello(a: 1.0, b: 12)

# custom pragma on iterators
# test routines
block:
template prag {.pragma.}
proc hello {.prag.} = discard
iterator hello2: int {.prag.} = discard
template hello3(x: int): int {.prag.} = x
macro hello4(x: int): int {.prag.} = x
func hello5(x: int): int {.prag.} = x
doAssert hello.hasCustomPragma(prag)
doAssert hello2.hasCustomPragma(prag)
doAssert hello3.hasCustomPragma(prag)
doAssert hello4.hasCustomPragma(prag)
doAssert hello5.hasCustomPragma(prag)

# test push doesn't break
block:
template prag {.pragma.}
{.push prag.}
proc hello = discard
iterator hello2: int = discard
template hello3(x: int): int = x
macro hello4(x: int): int = x
func hello5(x: int): int = x
type
Foo = enum a
Bar[T] = ref object of RootObj
x: T
case y: bool
of false: discard
else:
when true: discard
for a in [1]: discard a
{.pop.}

# issue #11511
when false:
Expand Down
23 changes: 23 additions & 0 deletions tests/pragmas/tinvalidcustompragma.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
discard """
cmd: "nim check $file"
"""

# issue #21652
type Foo = object
template foo() {.tags:[Foo].} = #[tt.Error
^ invalid pragma: tags: [Foo]]#
discard

{.foobar.} #[tt.Error
^ invalid pragma: foobar]#
type A = enum a {.foobar.} #[tt.Error
^ invalid pragma: foobar]#
for b {.foobar.} in [1]: discard #[tt.Error
^ invalid pragma: foobar]#
template foobar {.pragma.}
{.foobar.} #[tt.Error
^ cannot attach a custom pragma to 'tinvalidcustompragma'; custom pragmas are not supported for modules]#
type A = enum a {.foobar.} #[tt.Error
^ cannot attach a custom pragma to 'a'; custom pragmas are not supported for enum fields]#
for b {.foobar.} in [1]: discard #[tt.Error
^ cannot attach a custom pragma to 'b'; custom pragmas are not supported for `for` loop variables]#
7 changes: 3 additions & 4 deletions tests/stdlib/tsince.nim
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ doAssert ok
since (99, 3):
doAssert false

when false:
# pending bug #15920
# Error: cannot attach a custom pragma to 'fun3'
template fun3(): int {.since: (1, 3).} = 12
template fun3(): int {.since: (1, 3).} = 12

doAssert declared(fun3)