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

Default procedure parameters are untyped #12942

Open
liquidev opened this issue Dec 20, 2019 · 1 comment · Fixed by #24184
Open

Default procedure parameters are untyped #12942

liquidev opened this issue Dec 20, 2019 · 1 comment · Fixed by #24184

Comments

@liquidev
Copy link
Contributor

For some reason, default parameters of procedures are not checked semantically (typed), which causes problems eg. when you want to get their type. Nim does not provide another mechanism of getting the type of a default parameter of a proc, getType and getTypeInst are the only way, but (obviously) they expect nnkSym as their parameter node's kind.

Example

import macros

var thing = 2
proc example(a = thing) =
  discard (a, thing)

macro echoImpl(x: typed) = echo x.getImpl.treeRepr

echoImpl(example)

Current Output

ProcDef
  Sym "example"
  Empty
  Empty
  FormalParams
    Empty
    IdentDefs
      Ident "a"
      Empty
      Ident "thing"
  Empty
  Empty
  DiscardStmt
    TupleConstr
      Sym "a"
      Sym "thing"

Expected Output

ProcDef
  Sym "example"
  Empty
  Empty
  FormalParams
    Empty
    IdentDefs
      Ident "a"
      Empty
      Sym "thing"  # ← basically, run a semantic check on this
  Empty
  Empty
  DiscardStmt
    TupleConstr
      Sym "a"
      Sym "thing"

Possible Solution

  • Enable semantic checking on default params of procs.

Additional Information

$ nim -v
Nim Compiler Version 1.0.4 [Linux: amd64]
Compiled at 2019-11-27
Copyright (c) 2006-2019 by Andreas Rumpf

git hash: c8998c498f5e2a0874846eb31309e1d1630faca6
active boot switches: -d:release
@cooldome
Copy link
Member

cooldome commented Dec 24, 2019

PR welcome
It is actually should be:

ProcDef
  Sym "example"
  Empty
  Empty
  FormalParams
    Empty
    IdentDefs
      Sym "a"    # this a sym too
      Empty
      Sym "thing"  # ← basically, run a semantic check on this
  Empty
  Empty
  DiscardStmt
    TupleConstr
      Sym "a"
      Sym "thing"

The sempass doesn't replace idents with syms while it should. Changes to semProcTypeNode are required.

@Araq Araq closed this as completed in c21bf7f Sep 27, 2024
Araq pushed a commit that referenced this issue Sep 27, 2024
…24191)

Reverts #24184, reopens #12942, reopens #19118

#24184 seems to have caused a regression in
https://github.com/c-blake/thes and
https://github.com/c-blake/bu/blob/main/rp.nim#L84 reproducible with
`git clone https://github.com/c-blake/cligen; git clone
https://github.com/c-blake/thes; cd thes; nim c -p=../cligen thes`.
Changing the `const` to `let` makes it compile.

A minimization that is probably the same issue is:

```nim
const a: seq[string] = @[]

proc foo(x = a) =
  echo typeof(x)
  echo x

import macros

macro resemFoo() =
  result = getImpl(bindSym"foo")

block:
  resemFoo() # Error: cannot infer the type of parameter 'x'
```

This should be a regression test in a future reimplementation of #24184.
@narimiran narimiran reopened this Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants