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

fix subscript magic giving unresolved generic param type #23988

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Aug 20, 2024

fixes #19737

As in the diff, semResolvedCall sets the return type of a call to a proc to the type of the call. But in the case of the subscript magic, this type is the first generic param which is also supposed to be the type of the first argument, but this is invalid, the correct type is the element type eventually given by semSubscript. Some lines above also prevent the subscript magics from instantiating their params so this type ends up being an unresolved generic param.

Since the type of the node is not nil, prepareOperand doesn't try to type it again, and this unresolved generic param type ends up being the final type of the node. To prevent this, we just never set the type of the node if we encountered a subscript magic.

Maybe we could also rename the generic parameters of the subscript magics to stuff like DummyT, DummyI if we want this to be easier to debug in the future.

@Araq Araq added the merge_when_passes_CI mergeable once green label Aug 20, 2024
@Araq Araq closed this Aug 21, 2024
@Araq Araq reopened this Aug 21, 2024
@Araq Araq merged commit 04da0a6 into nim-lang:devel Aug 22, 2024
31 checks passed
Copy link
Contributor

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

Hint: mm: orc; opt: speed; options: -d:release
173614 lines; 8.641s; 654.48MiB peakmem

narimiran pushed a commit that referenced this pull request Dec 16, 2024
fixes #19737

As in the diff, `semResolvedCall` sets the return type of a call to a
proc to the type of the call. But in the case of the [subscript
magic](https://nim-lang.org/docs/system.html#%5B%5D%2CT%2CI), this type
is the first generic param which is also supposed to be the type of the
first argument, but this is invalid, the correct type is the element
type eventually given by `semSubscript`. Some lines above also [prevent
the subscript magics from instantiating their
params](https://github.com/nim-lang/Nim/blob/dda638c1ba985a77eac3c7518138992521884172/compiler/semcall.nim#L699)
so this type ends up being an unresolved generic param.

Since the type of the node is not `nil`, `prepareOperand` doesn't try to
type it again, and this unresolved generic param type ends up being the
final type of the node. To prevent this, we just never set the type of
the node if we encountered a subscript magic.

Maybe we could also rename the generic parameters of the subscript
magics to stuff like `DummyT`, `DummyI` if we want this to be easier to
debug in the future.

(cherry picked from commit 04da0a6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge_when_passes_CI mergeable once green
Projects
None yet
2 participants