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

Prohibit binding replacement in closed modules during precompile #57425

Merged
merged 1 commit into from
Feb 16, 2025

Conversation

Keno
Copy link
Member

@Keno Keno commented Feb 15, 2025

This applies the existing prohibition against introducing new bindings in a closed module to binding replacement as well (for the same reason - the change won't be persisted after reload). It is pretty hard to even reach this point, since eval into closed modules is already prohibited and there's no surface syntax for cross-module declaration, but it is technically reachable from lowered IR. Further, in the future we may make all of these builtins, which would make it easier. Thus, be consistent now and fully disallow binding replacement in closed modules during precompile.

This applies the existing prohibition against introducing new bindings
in a closed module to binding replacement as well (for the same reason -
the change won't be persisted after reload). It is pretty hard to
even reach this point, since `eval` into closed modules is already
prohibited and there's no surface syntax for cross-module declaration,
but it is technically reachable from lowered IR. Further, in the future
we may make all of these builtins, which would make it easier. Thus, be
consistent now and fully disallow binding replacement in closed modules
during precompile.
@Keno Keno merged commit 56aed62 into master Feb 16, 2025
5 of 7 checks passed
@Keno Keno deleted the kfsafebindingreplacement branch February 16, 2025 19:34
@KristofferC KristofferC added the backport 1.12 Change should be backported to release-1.12 label Feb 17, 2025
KristofferC pushed a commit that referenced this pull request Feb 17, 2025
)

This applies the existing prohibition against introducing new bindings
in a closed module to binding replacement as well (for the same reason -
the change won't be persisted after reload). It is pretty hard to even
reach this point, since `eval` into closed modules is already prohibited
and there's no surface syntax for cross-module declaration, but it is
technically reachable from lowered IR. Further, in the future we may
make all of these builtins, which would make it easier. Thus, be
consistent now and fully disallow binding replacement in closed modules
during precompile.

(cherry picked from commit 56aed62)
@KristofferC KristofferC mentioned this pull request Feb 17, 2025
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.12 Change should be backported to release-1.12
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants