Improve overloaded resolution for template with untyped parameter

[slangmgh]

The following code will generate compiler error:

import os

template with_temp_file(name: string, body: untyped) =
   block:
      let file {.inject.} = open(name, fmWrite)
      try:
         body
      finally:
         file.close

template with_temp_file(body: untyped) =
   with_temp_file(getTempDir() / "tempfile.txt", body)

when isMainModule:
   with_temp_file():
      file.write("hello")

The compiler output:

Error: undeclared identifier: 'file'

Because the compiler will first check the with_temp_file(name: string, body: untyped) for code with_temp_file(): ..., the first parameter is string, but the first argument is untyped code with undefined variable file.

We can improve the overloaded resolution rule with compare parameter count and argument count before check the argument type. If there is no default parameter value, then the argument count must equals to paramter count. For the previous example, the first with_temp_file takes 2 parameter, and the second with_temp_file take 1 parameter, so we need not to check the first template, and the code will compile successfully. For template with default paramter value, we can still use this rule to filter some template before check parameter type. Suppose the paramter count is T, and there is some parameter with default value, suppose number t, then the argument count must be between T - t and T.

[Araq]

Your proposal make sense but I think untyped parameters are an anti-feature and should be phased out entirely. But this is a long-term goal, in the meantime we could try your idea.

[Vindaar]

Your proposal make sense but I think untyped parameters are an anti-feature and should be phased out entirely. But this is a long-term goal, in the meantime we could try your idea.

I'm not sure I understand what the alternative to untyped parameters would be, given that huge amounts of macro logic require untyped (and not only because working with typed is sometimes difficult). Essentially all DSLs need untyped of some form or another, unless one wishes to define templates / procs for each thing that may be used in the DSL scope. And even that cannot work in many cases.

[Araq]

You would use typed and process typed ASTs, sometimes with nkIdent or nkError inside for unresolved identifiers and other sem'check problems. (And yes, I know that "typed" ASTs are currently quirky and under-developed and under-specified.)

[timotheecour]

Your proposal make sense but I think untyped parameters are an anti-feature and should be phased out entirely

i disagree but this should be discussed in its own dedicated RFC instead of scattering discussion on this in many places; 1 RFC = 1 topic (linking to an RFC is what should be used instead)

[timotheecour]

I don't think https://github.com/nim-lang/Nim/pull/18618 is the right fix; it doesn't help with this important case: optional params with a block trailing param, a very common pitfall that requires annoying workarounds (see https://github.com/nim-lang/Nim/issues/14346); in fact even the example motivating this RFC could be best re-written with a single overload (instead of 2) and optional params:

when true:
  template fn(a = 1, b = 2, body) = discard
  fn(1, 2):
    foo1
    foo2

  fn(1): # bug: doesn't work
    foo1
    foo2

IMO this is what should be fixed first, it would not only solve the problem described in PR description (even with the 2 overloads), but it will also work with optional params as shown here.

note

for non-block param, the following works, using named param:

template fn(a: int, body) = discard
template fn(body) = discard
fn(body = foo1)

possible implementation

in sigmatch, if the last param uses block syntax, constrain its position to be the last formal parameter

links

https://github.com/nim-lang/Nim/issues/17429 https://github.com/nim-lang/Nim/issues/17164 https://github.com/nim-lang/Nim/issues/9620 https://github.com/nim-lang/Nim/issues/14827 https://github.com/timotheecour/Nim/issues/630

[slangmgh]

@timotheecour Yes, I thought about it, with the case that have optional parameter(with default value) combine the last untyped param. Fix this need to change the parameter and argument bind algorithm: bind the first N arguments with first non-optional parameter, bind the last M arguments with last non-option parameter, then the middle arguments treated as optional paramter.

But I think it need to be handled with another RFC.

[slangmgh]

@timotheecour I think the PR nim-lang/Nim#18618 will fix lots of untyped parameter's undefined symbol compiler error with overloaded template, of case not all.

You can still improve this, but that's not the contradict with this PR.

[slangmgh]

@timotheecour I have an idea.

1. First map the index between arguments and parameters: bind first N non-optional parameter with the first N arguments and last M non-optional parameter with last M arguments, then the middle possible optional parameters.
2. If we cannot get the correct map, try next overloaded template.
3. Else check the type match with the maped argument and parameter, if there is undefined symbol, leave it until all argument is checked. If there is any failed match, treat this is not the correct match and try next overloaded template(So undefined symbol will not generated); if all other argument type is matched, and only some with undefined symbol unmatched, treat it as compiler error.

[timotheecour]

@timotheecour I have an idea.

I like it! can you open a separate PR for this? (you can leave the other PR open in the meantime); I expect this will make the other PR obsolete in the end as it's a more general fix.

The 2 phase approach is good (reject 1st based on param-argument index mismatch taking into account everything except the types of arguments, then if 1st succeeds, reject based on type mismatch, with special case for untyped params as optional further refinement)

if there is undefined symbol,

I think this is a refinement that can be done in a separate PR; we'll already get lots of benefits from the 2 step sigmatch. Checking for undefined symbols may not be robust, at least needs more thought.

[solo989]

Making default arguments work with untyped bodies sounds like it should be part of a completely seperate RFC.

Possibly combined with named only arguments support as a pragma or a new postfix operator so it doesn't break existing code.

[timotheecour]

Making default arguments work with untyped bodies sounds like it should be part of a completely seperate RFC.

here you go: https://github.com/nim-lang/RFCs/issues/405

Possibly combined with named only arguments support as a pragma or a new postfix operator so it doesn't break existing code.

i don't see a need for that, it shouldn't break code (please provide an example otherwise)

/cc @slangmgh looks like the idea you described in https://github.com/nim-lang/RFCs/issues/402#issuecomment-890289692 should be able to address https://github.com/nim-lang/RFCs/issues/405; a PR for that would be most welcome, it's a very common use case; I'd be happy to review and test

[metagn]

Working implementation of the described solution in the RFC as reference for future/just in case https://github.com/nim-lang/Nim/pull/21673

[metagn]

Ok well if we aren't implementing this we should at least document this limitation along with the workaround as outlined here.

[konsumlamm]

Can someone explain why this happens? Is file.write("hello") inferred as the string argument?

[metagn]

To resolve overloads, each argument in a call expression is iterated over, and if the argument type for an overload at some position is not untyped, that argument in the call expression is typechecked. This does not care about the total argument count of the overloads (although it could).

So if we have 2 overloads foo(untyped) and foo(untyped, untyped), any 1 or 2 arguments you pass to a call to foo will never be typed, while if you have foo(untyped) and foo(string, untyped), the 1st argument you pass will be typed, while the 2nd argument won't. Hence a workaround is to define private fooImpl(untyped) and fooImpl(untyped, string), and have visible aliases foo(a: untyped) = fooImpl(a) and foo(a, b: untyped) = fooImpl(b, a) that change the order of the arguments.

At this point we really need to document this.