expand OchoiceX on resem
#1110
Open
expand OchoiceX on resem
#1110
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We want it in general though. At least as an experiment. Maybe like Nim's typebound ops. |
Araq
pushed a commit
that referenced
this pull request
May 20, 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refs #1107 (makes issue test case work but isn't a substitute for argument dependent lookup in general)
OchoiceXwhen semchecked again (as is done for callee symbols in calls) now looks up the current context again for newly introduced symbols and adds them to the symchoice. This is not done in original Nim: instead of rewriting the symchoice node, open symchoices give new symbols during overload iteration. Doing it this way might unintentionally "accumulate" symbols after consecutive resems but I'm not sure that this would necessarily be a bad thing.To do it how original Nim does it, we can either only expand the symchoice if a callee semcheck flag is passed, or maybe
resolveOverloadswhen iterating overOchoiceXcan also look up newly introduced symbols. But maybe these are not general enough.Worth noting that this is the first difference in behavior between
OchoiceXandCchoiceXin the codebase.