Skip to content

isolate and rematch generic converters to get bindings #24867

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

Merged
merged 1 commit into from
Apr 12, 2025

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Apr 12, 2025

fixes #4554, fixes #10900, fixes #13843, fixes #19471, fixes #19517

Instead of matching generic converters to their arguments using the full call match bindings, a new match is created for them (from which the bindings are used to instantiate the converter return type). Then when instantiating generic converters, they are matched to their argument again to get their bindings again instead of using the call bindings. This prevents generic converters which match more than once from interfering with each other's bindings.

@Araq Araq merged commit 334f96c into nim-lang:devel Apr 12, 2025
18 checks passed
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 334f96c

Hint: mm: orc; opt: speed; options: -d:release
179257 lines; 8.895s; 652.703MiB peakmem

narimiran pushed a commit that referenced this pull request Apr 14, 2025
fixes #4554, fixes #10900, fixes #13843, fixes #19471, fixes #19517

Instead of matching generic converters to their arguments using the full
call match bindings, a new match is created for them (from which the
bindings are used to instantiate the converter return type). Then when
instantiating generic converters, they are matched to their argument
again to get their bindings again instead of using the call bindings.
This prevents generic converters which match more than once from
interfering with each other's bindings.

(cherry picked from commit 334f96c)
@borbek
Copy link

borbek commented Apr 22, 2025

The examples from wnim are not compiled. I suspect in connection with this change...

image

Araq pushed a commit that referenced this pull request Apr 24, 2025
refs #24867,
#24867 (comment)

The argument node of the converter can be wrapped in [hidden `addr` or
subtype conversion
nodes](https://github.com/nim-lang/Nim/blob/dc100c5caa673b039155e9e5d4c7fc0c239f4eb5/compiler/sigmatch.nim#L2327-L2335)
which have to be skipped when matching the type again, since the type of
the node is the uninstantiated type taken from the proc parameter.
narimiran pushed a commit that referenced this pull request May 5, 2025
refs #24867,
#24867 (comment)

The argument node of the converter can be wrapped in [hidden `addr` or
subtype conversion
nodes](https://github.com/nim-lang/Nim/blob/dc100c5caa673b039155e9e5d4c7fc0c239f4eb5/compiler/sigmatch.nim#L2327-L2335)
which have to be skipped when matching the type again, since the type of
the node is the uninstantiated type taken from the proc parameter.

(cherry picked from commit 8c9a645)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants