Skip to content

Prohibit binding replacement in closed modules during precompile #57425

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

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
24 tasks
KristofferC added a commit that referenced this pull request Feb 26, 2025
Backported PRs:
- [x] #57302 <!-- Add explicit imports for types and fix bugs -->
- [x] #57420 <!-- Compiler: Fix check for IRShow definedness -->
- [x] #57419 <!-- generated: Switch resolution module back to what it
was before -->
- [x] #57421 <!-- bpart: Skip implicit import reval if using'd export
set is unchanged -->
- [x] #57425 <!-- Prohibit binding replacement in closed modules during
precompile -->
- [x] #57426 <!-- Prohibit `import`ing or `using` Main during
incremental compilation -->
- [x] #57433 <!-- bpart: Track whether any binding replacement has
happened in image modules -->
- [x] #57445 <!-- Run all `--sysimage-native-code=no` cmdlineargs tests
single-threaded -->
- [x] #57386 <!-- Only strip invariant.load from special pointers -->
- [x] #57453 <!-- Revert "Make emitted egal code more loopy (#54121)"
-->
- [x] #57389 <!-- Change memory indexing to use the type as index
instead of i8 -->
- [x] #57447 <!-- Don't return null pointer when asking for the type of
a declared global -->
- [x] #57467 <!-- using/import: ensure world update after each
observable operation -->
- [x] #57471 <!-- staticdata: Don't use `newm` pointer after it has been
invalidated -->
- [x] #57416 <!-- lowering: Don't mutate lambda in `linearize` -->
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Feb 26, 2025
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.

2 participants