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

skip postfix node when resolving distinct type from impl AST #45

Merged
merged 1 commit into from
Dec 20, 2023
Merged

skip postfix node when resolving distinct type from impl AST #45

merged 1 commit into from
Dec 20, 2023

Conversation

metagn
Copy link
Contributor

@metagn metagn commented Dec 18, 2023

resolveTypeFromDistinct calls getImpl on the distinct type which yields an nkTypeDef node of the original distinct type declaration, and returns the first child of the node, what would normally be the type name.

Normally when a type is exported its name in the AST becomes an nkPostfix node, however due to a Nim bug this node was not saved in typed AST and the first child was always the type symbol node. If the bug in Nim is fixed, resolveTypeFromDistinct has to skip the postfix node here.

`resolveTypeFromDistinct` calls `getImpl` on the distinct type which yields an `nkTypeDef` node of the original distinct type declaration, and returns the first child of the node, what would normally be the type name.

Normally when a type is exported its name in the AST becomes an `nkPostfix` node, however due to a [Nim bug](nim-lang/Nim#22933) this node was not saved in typed AST and the first child was always the type symbol node. If the bug in Nim is fixed, `resolveTypeFromDistinct` has to skip the postfix node here.
@Vindaar
Copy link
Member

Vindaar commented Dec 20, 2023

Sorry for the delay! Should be less busy and more attentive again. I had no idea either that was a bug. Cool and thanks for the fix!

@Vindaar Vindaar merged commit a578232 into SciNim:master Dec 20, 2023
6 checks passed
Araq pushed a commit to nim-lang/Nim that referenced this pull request Dec 23, 2023
Continued from #23096 which was reverted due to breaking a package and
failing docgen tests. Docgen should now work, but ~~a PR is still
pending for the package: SciNim/Unchained#45
has been merged
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.

None yet

2 participants