A nested load inherits the enclosing file's compiler flags - #1085
Merged
Merged
Conversation
A file load brackets *warn-on-reflection*, *assert* and *unchecked-math*
so a top-level (set! *unchecked-math* true) -- the standard idiom in
ported libraries -- is legal and ends with the file. It bound them to the
vars' ROOTS. Compiler.load pushes WARN_ON_REFLECTION.deref(), the CURRENT
value, so on the reference a file loaded from inside another file
INHERITS the outer file's flags and stops inheriting only where that
outer frame ends.
Binding the root reset every nested load to the defaults instead.
Measured, same two files, jolt against the reference:
outer (set! *unchecked-math* true) (set! *warn-on-reflection* true)
(set! *assert* false), then loads inner three ways
unchecked-math warn-on-reflection assert
JVM load-string true true false
load-file true true false
require true true false
jolt all three false false true
So arithmetic under a require compiled differently from the arithmetic
above it, and a library that sets a flag at its top level had no effect
on anything it went on to load. Both agree the outer file keeps its own
values after the nested load returns, so only the inheritance direction
was wrong.
Two sites, because the value choice is made in two places:
* loader.ss ldr-with-file-vars -- load-file and a require from source.
* dyn-binding.ss jolt-ns-load-var-pairs -- the frame shared by
load-string (compile-eval.ss), a compiled namespace load
(ldr-with-compiled-ns-vars) and the entry points.
Both now read var-cell-deref, which is the .deref() this is meant to
mirror: the binding stack first, then the root.
The second one is also the OUTERMOST frame for an entry -- -e, a built
binary's -main -- and for the AOT replay it was originally written for.
Nothing is bound above those, so the current value there IS the root and
they read exactly as before; this is not a behaviour change for them.
run-sci.ss builds its own frame per FORM rather than per file, so a set!
there does not reach the next form of the same file. That is a separate
question about a build-time vendoring tool, not a user-facing load, and
it is left alone here.
Tests: six smoke rows, pinning the reference's actual answers rather than
what jolt's defaults were assumed to be -- all three flags inherited
through load-string, both directions in one form (the inner set! visible
to the rest of the inner load and gone once it returns), and a real file
through load-file with and without an outer frame. The existing rows for
the other direction -- a load's set! not escaping into the caller -- are
unchanged and still pass.
The file rows PRINT what the inner file saw instead of yielding it: the
first version read it back through load-file's return value and got
nothing, because jolt's load-file answers nil where the reference answers
the last form's value. That is a separate divergence, filed as jolt-5gx
and referenced from the rows so nobody folds them back onto the return.
load-string does answer its last form's value here, which is what the
load-string rows above rely on -- so the two disagree with each other as
well, and that is on the bead too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes bead jolt-8sf, found while reviewing #1079.
A file load brackets
*warn-on-reflection*,*assert*and*unchecked-math*so a top-level(set! *unchecked-math* true)— thestandard idiom in ported libraries — is legal and ends with the file. It
bound them to the vars' roots.
Compiler.loadpushesWARN_ON_REFLECTION.deref(), the current value, so on the reference afile loaded from inside another file inherits the outer file's flags and
stops inheriting only where that outer frame ends.
Binding the root reset every nested load to the defaults instead. Measured,
same two files, jolt against the reference — outer sets all three, then
loads inner three ways:
*unchecked-math**warn-on-reflection**assert*load-string/load-file/requireSo arithmetic under a
requirecompiled differently from the arithmeticabove it, and a library that set a flag at its top level had no effect on
anything it went on to load.
Both agree that the outer file keeps its own values after the nested load
returns, so only the inheritance direction was wrong. The existing rows
for the other direction — a load's
set!not escaping into its caller —are unchanged and still pass.
Two sites, not one
The bead named one. The value choice is made in two places, and between
them they cover all three entrances:
loader.ssldr-with-file-varsload-file,requirefrom sourcedyn-binding.ssjolt-ns-load-var-pairsload-string(compile-eval.ss), a compiled ns load (ldr-with-compiled-ns-vars), and the entry pointsBoth now read
var-cell-deref, which is the.deref()this is meant tomirror: the binding stack first, then the root. The functional change is
four lines.
The second site is also the outermost frame for an entry (
-e, a builtbinary's
-mainafter #1079) and for the AOT replay it was originallywritten for. Nothing is bound above those, so the current value there is
the root and they read exactly as before — this is not a behaviour change
for the boot path.
aotcachesmoke,devbootsmokeandgatebootsmokearein the gate list below for that reason.
Deliberately not in scope
run-sci.ssbuilds its frame per form rather than per file, so aset!there does not reach the next form of the same file. That is a separate
question about a build-time vendoring tool, not a user-facing load.
A second divergence the tests turned up
The file rows print what the inner file saw rather than yielding it. The
first version read it back through
load-file's return value — which ishow the reference behaves — and got nothing: jolt's
load-fileanswersnilwhere the reference answers the last form's value.load-stringdoes answer its last value here, so the two disagree with each other as
well as with the reference. Filed as jolt-5gx (P3) and referenced from
the rows so nobody folds them back onto the return value.
Verification
Six new smoke rows, pinned to the reference's measured answers rather than
to assumed defaults: all three flags inherited through
load-string; bothdirections in one form (the inner
set!visible to the rest of the innerload and gone once it returns); and a real file through
load-filewithand without an outer frame.
Green: smoke (225 → 231), unit, loaderconf 33/33, deadhost, portcheck,
aotcachesmoke 26/26, devbootsmoke 7/7, gatebootsmoke 10/10, buildsmoke (35
scenarios).