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

Make ctx.pass immutable #11538

Merged
merged 9 commits into from Feb 5, 2024
Merged

Make ctx.pass immutable #11538

merged 9 commits into from Feb 5, 2024

Conversation

Simn
Copy link
Member

@Simn Simn commented Feb 2, 2024

This was easier than expected. We now also clone the context before typing expressions, which means that we don't have to mutate ctx.pass <- PTypeField anymore. This in turn means that all context instances now have a fixed pass, which should make everything much easier to debug. We could even give them names and go to their piano rehearsals.

I'd like to see if I can break the macro change here because I feel like this should be breakable, given that we no longer enter PTypeField pass. I think this portion of code should also clone a new expression context, but before I do that I'll see how to actually trigger an issue here.

@Simn
Copy link
Member Author

Simn commented Feb 2, 2024

This indeed fails exactly like I expected it to: a build macro causing the typing of a module, and then trying to access a class field doesn't work because we never flush PBuildClass. It's a bit scary that we didn't test this before, but at least the typer passes behave predicable for once.

# Conflicts:
#	src/typing/macroContext.ml
@kLabz
Copy link
Contributor

kLabz commented Feb 4, 2024

Wartales breaks badly with this change :/ (will send errors via slack tomorrow morning)

Other projects seem mostly ok (so it may be about hxbit closures?), ie. as ok as they are with current nightlies (so need to revert @:bypassAccessor and bind changes in most projects, and there are some old (pre-hxb at least) issues with complex recordings; but results are the same as with dev for replays and compilation is ok).

@Simn Simn merged commit 3f13b75 into development Feb 5, 2024
121 of 122 checks passed
@Simn Simn deleted the more_typer_cleanup branch February 5, 2024 07:23
@Simn
Copy link
Member Author

Simn commented Feb 5, 2024

For documentation purposes: the problem was that ctx.allow_inline and ctx.allow_transform weren't inherited by "child" contexts. The expected state appears to be that they're reset only when creating a new module's context.

@skial skial mentioned this pull request Feb 6, 2024
1 task
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