Skip to content

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented May 18, 2025

closes #1107

Same as the implementation in original Nim, if an argument has a nominal type and the nominal type is exported from a module, the toplevel symbols of the module the type is from are scanned if they are an attachable routine to the nominal type, in which case they are considered in overload resolution.

This is done in the same place as concept procs, they are now built directly in considerTypeboundOps as well by iterating over cs.args rather than adding them when the arguments are typechecked. This is to simplify preventing symbols that were already considered in overloading from getting added, they now use the same marker set as the added typebound ops.

Apparently the position of the argument is not considered in attachment, every parameter of the routine is checked for the nominal type. Presumably this is to simplify the logic for things like varargs and named arguments. It apparently also ignores parameters with default values. This logic is unchanged.

Something to point out is that attached routines for types from the current semchecked module have to be special cased. One might assume they shouldn't be added at all since it would be redundant with regular lookup, but then the original test case in #1107 would not work without a fix like in #1110, as regular lookup with OchoiceX does not work as expected yet. Another problem with adding attached routines from the current module is that using a cache would not be reliable, and so it is not done here, but this means we have to search for attached routines every time for them.

The original Nim implementation also disables type bound ops when the callee is a closed symbol/symchoice. This could be done here for open symchoices but it would be a little arbitrary to track the case where the callee is originally an identifier that gets turned into a symbol, maybe it can produce an open symchoice but this would complicate type conversions etc. So nothing is done for now.

@metagn metagn marked this pull request as ready for review May 19, 2025 13:16
@metagn metagn mentioned this pull request May 19, 2025
@Araq
Copy link
Member

Araq commented May 19, 2025

So if I read the test case correctly, this solution makes this program work

import std / syncio

type
  MyS = distinct string

proc write*(f: File; s: MyS) = write f, string(s)

proc main =
  var s90 = MyS"abc"
  echo s90

main()

and yet does not depend on the open vs closed sym choice feature. Correct? If so, that's exactly what I was looking for!

@metagn
Copy link
Collaborator Author

metagn commented May 19, 2025

It does but it's special cased, looking up top level symbols in the current module needs different code (the moduleSuffix == c.thisModuleSuffix branch), and it doesn't use the cache. It's also only for routines that can be attached to nominal types.

It also always adds the typebound ops to overload resolution even if the callee is a sym or closed symchoice, if this causes problems we would still need a distinction with open symchoices/idents.

@Araq Araq merged commit fffc219 into nim-lang:master May 20, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

we need argument dependent lookups
2 participants