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

Python: Fix custom types generating invalid code when there are forward references #2075

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

mhammond
Copy link
Member

Also includes many other custom-type tests, all of which work, and clarifies in the docs about which types can be used with custom types.

Fixes #2067

@mhammond mhammond requested a review from a team as a code owner April 15, 2024 19:12
@mhammond mhammond requested review from badboy and removed request for a team April 15, 2024 19:12
@mhammond mhammond force-pushed the python-forward-references branch 2 times, most recently from e574c6c to 9d81c65 Compare April 15, 2024 20:07
Copy link
Contributor

@bendk bendk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, love all the tests. Just had one doc suggestion about the ordering algorithm.

None => ordered.push((name, builtin)),
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could maybe fail if there were multiple levels of builtins, eg:

A { builtin: C},
B { }
C { builtin: B }

would get sorted to

C { builtin: B }
A { builtin: C},
B { }

That seems pretty pathological for custom types and I think it's probably fine to ignore. How about adding a note mentioning this? That way we don't repurpose the algo for a situation where it would matter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, thanks!

…rd references.

Also includes many other custom-type tests, all of which work, and clarifies
in the docs about which types can be used with custom types.

Fixes mozilla#2067
@mhammond mhammond force-pushed the python-forward-references branch from 9d81c65 to b97c6dd Compare April 19, 2024 01:41
@mhammond mhammond merged commit f0e5d42 into mozilla:main Apr 19, 2024
5 checks passed
@bendk bendk mentioned this pull request May 14, 2024
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.

Python type aliases don't use forward references
2 participants