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

Improve passing parameters to macros as statement #741

Closed
gradha opened this issue Dec 12, 2013 · 5 comments
Closed

Improve passing parameters to macros as statement #741

gradha opened this issue Dec 12, 2013 · 5 comments
Assignees

Comments

@gradha
Copy link
Contributor

gradha commented Dec 12, 2013

The following example:

macro params*(a: string, b = "aa", body: stmt): stmt {.immediate.} =
  echo "Param a is ", repr(a)
  echo "Param b is ", repr(b)
  echo "The body is ", repr(body)

when isMainModule:
  params(4):
    let there = "be peace"

shows several wrong things when compiled:

Param a is 4
Param b is 
  let there = "be peace"
The body is stack trace: (most recent call last)
mamamama.nim(7)          params
mamamama.nim(4, 28) Error: cannot evaluate '
  echo "Param a is ", repr(a)
  echo "Param b is ", repr(b)
  echo "The body is ", repr(body)'

There are several things which could be improved:

  • The macro defines parameters a/b to be of string type, but an integer is happily passed. The compiler could try to enforce the correct types if the macro defines to specify them as specific times instead of stmt or PNimNode so that the implementor of the macro doesn't have to check himself for the proper type.
  • The usage of the macro as a statement could modify how parameters and default parameters are being handled, with the body being passed always as the last parameter. This would allow specifying default values for the other parameters starting from the previous to last.
  • Current default parameters are ignored (or are overwritten with the body?)
  • No checks at all seem to be done for the number of parameters being passed to the macro, hence the strange stack crash of the compiler evaluating body since it is actually empty and the body is being passed as the b parameter.
@fowlmouth
Copy link
Contributor

This is because the macro is {.immediate.}
"There are two different kinds of templates: immediate templates and ordinary templates. Ordinary templates take part in overloading resolution. As such their arguments need to be type checked before the template is invoked. So ordinary templates cannot receive undeclared identifiers"

@gradha
Copy link
Contributor Author

gradha commented Dec 12, 2013

That's sort of what this feature request would be: even if {.immediate.} is used and the macro doesn't take part in overloading resolution, would it be feasible to check types other than generic expr once the compiler has decided to use that macro? It's like the programmer telling the compiler to go the extra mile and please do check those parameters to avoid having to error check them yourself.

Ignoring that, even without {.immediate.} the compiler ignores the default values for the parameters which may be a useful feature to have. Or not.

@zah
Copy link
Member

zah commented Dec 13, 2013

Well, we have plans to gradually phase out the need for {.immediate.}. In such a world, only the stmt and expr params won't be type checked immediately (the immediate-ness becomes a property of the param and not the macro itself).

The other proposal for special treatment of calls featuring both default and stmt params is something that have been considered and I agree that it seems quite intuitive for the user. It's even possible to allow default parameters to appear anywhere in the signature and the matching algorithm to work as a DFA (i.e. like a regular expression such as "a b? c"), but obviously this will be quite an unusual feature.

@ghost ghost assigned zah Jan 23, 2014
@genotrance
Copy link
Contributor

Latest nim #head complains because b is not passed but body is which is invalid. Also, {.immediate.} is reported as deprecated.

I also tried removing b and it compiles so it still accepts the integer value. Does this deserve to be closed yet?

Hint: used config file 'c:\Users\gt\Desktop\dl\programming\nimdevel\config\nim.cfg' [Conf]
Hint: system [Processing]
Hint: temp [Processing]
temp.nim(1, 42) Warning: stmt is deprecated [Deprecated]
temp.nim(1, 49) Warning: stmt is deprecated [Deprecated]
temp.nim(1, 54) Warning: use 'untyped' parameters instead; immediate is deprecated [Deprecated]
temp.nim(7, 9) Error: in call 'params(4):
  let there = "be peace"' got 2, but expected 3 argument(s)
Traceback (most recent call last)
nim.nim(121)             nim
nim.nim(77)              handleCmdLine
main.nim(173)            mainCommand
main.nim(77)             commandCompileToC
modules.nim(242)         compileProject
modules.nim(182)         compileModule
passes.nim(250)          processModule
passes.nim(139)          processTopLevelStmt
sem.nim(576)             myProcess
sem.nim(544)             semStmtAndGenerateGenerics
semstmts.nim(1929)       semStmt
semexprs.nim(797)        semExprNoType
semexprs.nim(2319)       semExpr
semexprs.nim(1943)       semWhen
semexprs.nim(2396)       semExpr
semstmts.nim(1862)       semStmtList
semexprs.nim(2285)       semExpr
sem.nim(441)             semMacroExpr
vm.nim(1711)             evalMacroCall
msgs.nim(1049)           globalError
msgs.nim(1037)           liMessage
msgs.nim(891)            handleError
msgs.nim(876)            quit

@Araq
Copy link
Member

Araq commented Apr 21, 2018

Nice feature, but not gonna happen anytime soon. Closing.

@Araq Araq closed this as completed Apr 21, 2018
Clyybber pushed a commit to Clyybber/Nim that referenced this issue Sep 16, 2023
## Summary

Introduce a set (`codegenExprNodeKinds`) with all node kinds that can
appear
in *general* expression and statement contexts during the code
generation phase. The set is then used to make the `case` statement in
the main AST traversal procedures exhaustive (no more catch-all `else`
branch).

## Details

The set doesn't include node kinds (such as `nkExprColonExpr` or
`nkElse`) that can only appear in nested contexts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants