Skip to content

[TODO] fix #13524: magics with untyped params (eg astToStr) now work inside generics #13525

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

Closed
timotheecour wants to merge 2 commits into from
Closed

[TODO] fix #13524: magics with untyped params (eg astToStr) now work inside generics #13525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ed4ae2
fix #13524: magics with untyped params (eg astToStr) now work inside …
timotheecour Feb 28, 2020
090a418
address comments
timotheecour Mar 18, 2020
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
12 changes: 11 additions & 1 deletion compiler/semgnrc.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,14 @@ proc semGenericStmt(c: PContext, n: PNode,
var mixinContext = false
if s != nil:
incl(s.flags, sfUsed)
mixinContext = s.magic in {mDefined, mDefinedInScope, mCompiles}
# more robust/future proof than:
# mixinContext = s.magic in {mDefined, mDefinedInScope, mCompiles, mAstToStr}
if s.magic != mNone and s.typ != nil:
for i in 1..<s.typ.len:
if s.typ[i].kind == tyUntyped:
mixinContext = true
break

let sc = symChoice(c, fn, s, if s.isMixedIn: scForceOpen else: scOpen)
case s.kind
of skMacro:
Expand Down Expand Up @@ -283,6 +290,9 @@ proc semGenericStmt(c: PContext, n: PNode,
# is not exported and yet the generic 'threadProcWrapper' works correctly.
let flags = if mixinContext: flags+{withinMixin} else: flags
for i in first..<result.safeLen:
# instead, would be better to only set `withinMixin` for arguments of
# kind tyUntyped, eg `if s.typ[i].kind == tyUntyped:`, however, this
# currently doesn't work
result[i] = semGenericStmt(c, result[i], flags, ctx)
of nkCurlyExpr:
result = newNodeI(nkCall, n.info)
Expand Down
2 changes: 1 addition & 1 deletion lib/system.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1629,7 +1629,7 @@ else:
result[i+1] = y[i]


proc astToStr*[T](x: T): string {.magic: "AstToStr", noSideEffect.}
proc astToStr*(x: untyped): string {.magic: "AstToStr", noSideEffect.}
Copy link
Member

@Araq Araq Feb 28, 2020

Choose a reason for hiding this comment

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

This is a breaking change though, for example, now astToStr(2 + 3) produces something different. Arguably something better, but ... breaking change!

Copy link
Member Author

@timotheecour timotheecour Feb 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

not sure what you mean; both before and after PR I get:

echo 'echo astToStr(2+3)' | nim c -r -f -
2 + 3

(where the -f is needed until #13506 gets merged)

x is really untyped, not typed, so the previous declaration didn't make sense (in particular, T is meaningless); it only worked because of the way magics currently work, but hopefully one day this would be an error

## Converts the AST of `x` into a string representation. This is very useful
## for debugging.

Expand Down
6 changes: 6 additions & 0 deletions tests/generics/t13525.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# https://github.com/nim-lang/Nim/issues/13524
template fun(field): untyped = astToStr(field)
proc test1(): string = fun(nonexistant1)
proc test2[T](): string = fun(nonexistant2) # used to cause: Error: undeclared identifier: 'nonexistant2'
doAssert test1() == "nonexistant1"
doAssert test2[int]() == "nonexistant2"