Skip to content

Commit 73576bb

Browse files
committed
Implement mechanism for strict mode
This implements support for the `strict mode` mechanism proposed in #54903. There was a desire to see an implementation of this along side #60018 since that are conceptually adjacent. However, other than some shared infrastructure, this version is farther from #60018 than the original proposal for reasons mentioned below. The motivation and basic idea are largely unchanged. # Implemented semantics The user facing interface to this mechanism is `@strict` (currently in `Base.Experimental`, but expected to be moved out in the course of the 1.14 release process. Rather than paraphrase, let me just provide the first part of the docstring that describes the semantics: ``` Set the `strict` mode for the current module to all flags specified in `flags`. Each `strict` mode flag is an independent option that disallows certain syntax or semantics that may be undesirable in *some* contexts. Package authors should consider which flags are appropriate for their package and set them at the top of the applicable module. # General semantics and philosophy Note that additional strict mode flags are not necessarily *safer* or *better* in any way; they are a reflection of of the reality that different users have different tradeoffs for their codebases and what may be a sensible restriction in a mature package could be very annoying in the REPL. As designed, there are several general guidelines that apply to strict mode flags. To the extent possible, they should be kept for future flag additions. 1. Code that evaluates without error in strict mode should also evaluate without error under the ordinary julia execution semantics. 2. Strict mode should not affect parsing. If it is desirable to disallow a particular syntax pattern, it should be recognized at the lowering stage. If this is currently not possible, the parser should be modified to emit an appropriate marker that can be checked at lowering time. 3. Strict mode is not intended for for issues that are clearly bugs. Those should instead use the syntax versioning mechanism (see [`Base.Experimental.@set_syntax_version`](@ref)). However, `strict` mode flags that gain widespread adoption may eventually be considered as candidates for syntax evolution. Strict mode flags are automatically inherited by submodules, but can be overriden by an explicit `@strict` invocation in the submodule. Strict mode flags are partitioned by world age. # Specifying flags The `flags` expression is runtime-evaluated and should evaluate to a collection of `Symbols` as specified below. In addition, nested collections of symbols are allowed and will be flattened. This is intended to support specifying strict mode flags in a central location and enforcing them across multiple dependents. ``` # Module vs Project.toml opt-ins Of particular note is that I ended up deciding on a per-module opt-in rather than a Project.toml opt in (like was originally proposed in #54903, and is implemented for syntax evolution in #60018). This is for the following reasons: 1. The #60018 experience has shown that project.toml opt ins are semantically somewhat awkward and need to be implemented both in the language and the package manager. This was fine for the syntax version, but strict mode is richer (and potentially much richer in the future) and adding this complexity into code loading seems undesirable. 2. One of the design objectives is to allow user-defined collections of strict mode flags enforced centrally across multiple packages. In this design this is easy by having a MyOrganizationBase package that defines a variable with the set of flags to enable. Doing something like this in Project.toml opens a whole can of worms on how to represent that. 3. I believe the concern about wanting to enable parse-time strict mode can be adequately addressed by having the parser emit a special marker that can then get picked up and checked against the strict mode by lowering (such marker addition possibly making use of the syntax evolution mechanism). If this is not how it works, the parser would need additional input state specifying the strict mode flags. #60018 has shown that changing parser state flags dynamically is undesirable, because people don't have a good sense of what the parse unit is. As such, I don't want the parser to look at strict mode flags at all. 4. As implemented here `@strict` inherits binding-world-age semantics. Since these are now well defined as of 1.12, this addresses a lot of the ordering concerns that were brought up in the discussion of #54903. 5. I think it may be useful to opt into certain strict mode flags for some modules in a package only (unlike the syntax version, where I don't expect this to be common). E.g. packages may define modules that define their core API or segregate their core algorithms from support code and may want more strict coding styles for such core modules. There remains a bit of a concern that this is less friendly to IDEs. I'm sympathetic to that, but the analysis required to compute the strict mode is a lot simpler than other analyses (so a language server should easily be able to do that) and I think it's outweighed particularly by the desire for user-definable collections (which requires the IDE to do some sort of analysis of however that is specified anyway). Given that, I think this mechanism is as IDE friendly as it gets, since the required capability is simply to compute the value of a constant obtained from a macro expansion (so no special strict-mode specific analysis required). # Implementation details The core of the implementation is simple, to determine the active strict mode flags, we simply look up the `_internal_module_strict_flags` binding in the appropriate module and see which flags are set. The exact types and values of this binding are explicitly and intentionally implementation details and `@strict` decides how to set it. This is inteded to allow flexibility of implementation in the future here. To faciliate the above described semantics of `@strict`, this binding has a couple of special features: 1. It gets automatically imported from a module's parent module upon module creation. 2. Unlike bindings created through syntax, invalidations from imports to `const` is permitted. Otherwise the mechanism behaves as an ordinary binding, including obeying world age semantics, and being Revise-able, etc. In particular, if you Revise the `@strict` setting in a top-level module it will automatically (through binding invalidation) be updated in all submodules. It was important to me that doing this would not leave the settings inconsistent. # Implemented strict mode flags Two strict mode flags are implemented in this PR, but they should largely be considered straw-man implementations to show how to access the flags set from within either the runtime or lowering. Everything works end-to-end, but we may want to do some extra work refining the precise semantics of these flags once we've merged the core mechanism. The reason for choosing these flags is simply that they were easy to implement. Several of the other proposed flags would require additional analysis in lowering, which should go in their own PRs. Implemented flags are as follows: ``` * `typeimports` This flag turns the 1.12 warning for implicit import of types into an error. Note that the implicit import default may be removed in a future Julia syntax iteration, in which case this flag will become a no-op for such versions. ```jldoctest julia> @Base.Experimental.strict :typeimports julia> String(x) = 1 ERROR: `@strict :typeimports` disallows extending types without explicit import in TypeImports: function Base.String must be explicitly imported to be extended ``` * `:nointliteraliterators` Disallows (at the lowering stages) literal integers as iterators in `for` loops. This protects against expressions like `for i in 10` which are commonly intended to be `for i in 1:10`. ```jldoctest julia> for i in 10 println(i) end 10 julia> @Base.Experimental.strict :nointliteraliterators julia> for i in 10 println(i) end ERROR: syntax: `@strict :nointliteraliterators` disallows integer literal iterators here around none:1 Stacktrace: [1] top-level scope @ none:1 ```
1 parent ade1dc4 commit 73576bb

File tree

17 files changed

+246
-38
lines changed

17 files changed

+246
-38
lines changed

base/Base_compiler.jl

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
module Base
44

5-
Core._import(Base, Core, :_eval_import, :_eval_import, true)
6-
Core._import(Base, Core, :_eval_using, :_eval_using, true)
5+
Core._import(Base, Core, :_eval_import, :_eval_import, 0x1)
6+
Core._import(Base, Core, :_eval_using, :_eval_using, 0x1)
77

88
using .Core.Intrinsics, .Core.IR
99

@@ -148,6 +148,12 @@ function _setup_module!(mod::Module, Core.@nospecialize syntax_ver)
148148
Core._using(mod, _topmod(mod), UInt8(0))
149149
Core.declare_const(mod, :include, IncludeInto(mod))
150150
Core.declare_const(mod, :eval, Core.EvalInto(mod))
151+
parent = ccall(:jl_module_parent, Ref{Module}, (Any,), mod)
152+
if parent === mod
153+
else
154+
Core._import(mod, parent, :_internal_module_strict_flags,
155+
:_internal_module_strict_flags, 0x3)
156+
end
151157
if syntax_ver === nothing
152158
return nothing
153159
end

base/boot.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,7 @@ let
726726
a = getfield(paths, i).args
727727
length(a) == 1 || fail()
728728
s = getindex(a, 1)
729-
Core._import(to, from, s, s, explicit)
729+
Core._import(to, from, s, s, explicit ? 0x1 : 0x0)
730730
i += 1
731731
end
732732
end

base/docs/basedocs.jl

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2782,7 +2782,7 @@ See also [`global`](@ref), [`setglobal!`](@ref), [`get_binding_type`](@ref Core.
27822782
Core.declare_global
27832783

27842784
"""
2785-
declare_const(module::Module, name::Symbol, [x])
2785+
declare_const(module::Module, name::Symbol, [x, [flags::UInt8]])
27862786
27872787
Create or replace the constant `name` in `module` with the new value `x`. When
27882788
replacing, `x` does not need to have the same type as the original constant.
@@ -2817,11 +2817,14 @@ See also [`const`](@ref).
28172817
Core.declare_const
28182818

28192819
"""
2820-
_import(to::Module, from::Module, asname::Symbol, [sym::Symbol, imported::Bool])
2820+
_import(to::Module, from::Module, asname::Symbol, [sym::Symbol, flags::Uint8])
28212821
28222822
With all five arguments, imports `sym` from module `from` into `to` with name
2823-
`asname`. `imported` is true for bindings created with `import` (set it to
2824-
false for `using A: ...`).
2823+
`asname`. `flags` is a bitmask of JL_IMPORT_FLAG_* flags.
2824+
- JL_IMPORT_FLAG_EXPLICIT is set for bindings created with `import` (unset for `using A: ...`).
2825+
- JL_IMPORT_FLAG_ALLOW_UNDEF allows the imported binding to be undefined at
2826+
import time (this is ordinarily a warning). Note that the binding may be
2827+
resolved later if it becomes available in the imported module.
28252828
28262829
With only the first three arguments, creates a binding for the module `from`
28272830
with name `asname` in `to`.

base/experimental.jl

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,4 +841,111 @@ function var"@VERSION"(__source__::Union{LineNumberNode, Core.MacroSource}, __mo
841841
end
842842
end
843843

844+
# N.B.: Must match the values in julia_internal.h
845+
const JL_STRICT_IMPORT_TYPE = 0x01
846+
const JL_STRICT_LITERAL_ITERATORS = 0x02
847+
848+
function strict_mode_flags(flag::Symbol)
849+
if flag === :typeimports
850+
return JL_STRICT_IMPORT_TYPE
851+
elseif flag === :nointliteraliterators
852+
return JL_STRICT_LITERAL_ITERATORS
853+
else
854+
error("unknown strict mode flag: $flag")
855+
end
856+
end
857+
strict_mode_flags(flags) =
858+
mapreduce(strict_mode_flags, |, flags; init=0x0)
859+
860+
function set_strict_mode(mod::Module, @nospecialize(flags))
861+
Core.declare_const(mod, :_internal_module_strict_flags,
862+
strict_mode_flags(flags), Base.JL_CONST_MAY_REPLACE_IMPORTS)
863+
return nothing
864+
end
865+
866+
"""
867+
Base.Experimental.@strict flags
868+
869+
Set the `strict` mode for the current module to all flags specified in `flags`.
870+
Each `strict` mode flag is an independent option that disallows certain syntax or
871+
semantics that may be undesirable in *some* contexts. Package authors should
872+
consider which flags are appropriate for their package and set them at the top
873+
of the applicable module.
874+
875+
# General semantics and philosophy
876+
877+
Note that additional strict mode flags are not necessarily *safer* or *better*
878+
in any way; they are a reflection of of the reality that different users have
879+
different tradeoffs for their codebases and what may be a sensible restriction
880+
in a mature package could be very annoying in the REPL.
881+
882+
As designed, there are several general guidelines that apply to strict mode flags.
883+
To the extent possible, they should be kept for future flag additions.
884+
885+
1. Code that evaluates without error in strict mode should also evaluate without
886+
error under the ordinary julia execution semantics.
887+
888+
2. Strict mode should not affect parsing. If it is desirable to disallow a
889+
particular syntax pattern, it should be recognized at the lowering stage.
890+
If this is currently not possible, the parser should be modified to emit an
891+
appropriate marker that can be checked at lowering time.
892+
893+
3. Strict mode is not intended for for issues that are clearly bugs.
894+
Those should instead use the syntax versioning mechanism (see
895+
[`Base.Experimental.@set_syntax_version`](@ref)). However, `strict` mode flags
896+
that gain widespread adoption may eventually be considered as candidates for
897+
syntax evolution.
898+
899+
Strict mode flags are automatically inherited by submodules, but can be
900+
overriden by an explicit `@strict` invocation in the submodule. Strict mode
901+
flags are partitioned by world age.
902+
903+
# Specifying flags
904+
905+
The `flags` expression is runtime-evaluated and should evaluate to a collection
906+
of `Symbols` as specified below. In addition, nested collections of symbols are
907+
allowed and will be flattened. This is intended to support specifying strict
908+
mode flags in a central location and enforcing them across multiple dependents.
909+
910+
# Currently supported flags
911+
912+
* `typeimports`
913+
914+
This flag turns the 1.12 warning for implicit import of types into an error.
915+
Note that the implicit import default may be removed in a future Julia syntax
916+
iteration, in which case this flag will become a no-op for such versions.
917+
918+
```jldoctest
919+
julia> @Base.Experimental.strict :typeimports
920+
921+
julia> String(x) = 1
922+
ERROR: `@strict :typeimports` disallows extending types without explicit import in TypeImports: function Base.String must be explicitly imported to be extended
923+
```
924+
925+
* `:nointliteraliterators`
926+
927+
Disallows (at the lowering stages) literal integers as iterators in `for` loops.
928+
This protects against expressions like `for i in 10` which are commonly intended
929+
to be `for i in 1:10`.
930+
931+
```jldoctest
932+
julia> for i in 10
933+
println(i)
934+
end
935+
10
936+
937+
julia> @Base.Experimental.strict :nointliteraliterators
938+
939+
julia> for i in 10
940+
println(i)
941+
end
942+
ERROR: syntax: `@strict :nointliteraliterators` disallows integer literal iterators here around none:1
943+
Stacktrace:
944+
[1] top-level scope
945+
@ none:1
946+
"""
947+
macro strict(flags)
948+
Expr(:call, set_strict_mode, __module__, esc(flags))
949+
end
950+
844951
end # module

base/module.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ function _eval_import(imported::Bool, to::Module, from::Union{Expr, Nothing}, pa
113113
if name !== nothing
114114
asname = asname === nothing ? name : asname
115115
check_macro_rename(name, asname, keyword)
116-
Core._import(to, m, asname, name, imported)
116+
Core._import(to, m, asname, name, imported ? JL_IMPORT_FLAG_EXPLICIT : 0x0)
117117
else
118118
Core._import(to, m, asname === nothing ? nameof(m) : asname)
119119
end

base/runtime_internals.jl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,11 @@ const BINDING_FLAG_ANY_IMPLICIT_EDGES = 0x8
262262

263263
const JL_MODULE_USING_REEXPORT = 0x1
264264

265+
const JL_IMPORT_FLAG_EXPLICIT = 0x1
266+
const JL_IMPORT_FLAG_ALLOW_UNDEF = 0x2
267+
268+
const JL_CONST_MAY_REPLACE_IMPORTS = 0x1
269+
265270
is_defined_const_binding(kind::UInt8) = (kind == PARTITION_KIND_CONST || kind == PARTITION_KIND_CONST_IMPORT || kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_BACKDATED_CONST)
266271
is_some_const_binding(kind::UInt8) = (is_defined_const_binding(kind) || kind == PARTITION_KIND_UNDEF_CONST)
267272
is_some_imported(kind::UInt8) = (kind == PARTITION_KIND_IMPLICIT_GLOBAL || kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_EXPLICIT || kind == PARTITION_KIND_IMPORTED)

src/ast.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ typedef struct _jl_ast_context_t {
3939
value_t ssavalue_sym;
4040
value_t slot_sym;
4141
jl_module_t *module; // context module for `current-julia-module-counter`
42+
uint8_t module_strict_flags; // cached strict mode flags for module
4243
struct _jl_ast_context_t *next; // invasive list pointer for getting free contexts
4344
} jl_ast_context_t;
4445

@@ -151,11 +152,19 @@ static value_t fl_julia_scalar(fl_context_t *fl_ctx, value_t *args, uint32_t nar
151152
return fl_ctx->F;
152153
}
153154

155+
static value_t fl_module_strict_flags(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
156+
{
157+
argcount(fl_ctx, "julia-current-module-flags", nargs, 0);
158+
jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx);
159+
return fixnum(ctx->module_strict_flags);
160+
}
161+
154162
static jl_value_t *scm_to_julia_(fl_context_t *fl_ctx, value_t e, jl_module_t *mod);
155163

156164
static const builtinspec_t julia_flisp_ast_ext[] = {
157165
{ "defined-julia-global", fl_defined_julia_global }, // TODO: can we kill this safepoint
158166
{ "current-julia-module-counter", fl_module_unique_name },
167+
{ "julia-current-module-flags", fl_module_strict_flags },
159168
{ "julia-scalar?", fl_julia_scalar },
160169
{ NULL, NULL }
161170
};
@@ -181,6 +190,7 @@ static void jl_init_ast_ctx(jl_ast_context_t *ctx) JL_NOTSAFEPOINT
181190
ctx->ssavalue_sym = symbol(fl_ctx, "ssavalue");
182191
ctx->slot_sym = symbol(fl_ctx, "slot");
183192
ctx->module = NULL;
193+
ctx->module_strict_flags = 0;
184194
set(symbol(fl_ctx, "*scopewarn-opt*"), fixnum(jl_options.warn_scope));
185195
}
186196

@@ -204,13 +214,16 @@ static jl_ast_context_t *jl_ast_ctx_enter(jl_module_t *m) JL_GLOBALLY_ROOTED JL_
204214
jl_init_ast_ctx(ctx);
205215
}
206216
ctx->module = m;
217+
if (m)
218+
ctx->module_strict_flags = jl_module_strict_flags(m, jl_current_task->world_age);
207219
return ctx;
208220
}
209221

210222
static void jl_ast_ctx_leave(jl_ast_context_t *ctx)
211223
{
212224
uv_mutex_lock(&flisp_lock);
213225
ctx->module = NULL;
226+
ctx->module_strict_flags = 0;
214227
ctx->next = jl_ast_ctx_freed;
215228
jl_ast_ctx_freed = ctx;
216229
uv_mutex_unlock(&flisp_lock);

src/ast.scm

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -489,12 +489,13 @@
489489
(define vinfo:type cadr)
490490
(define (vinfo:set-type! v t) (set-car! (cdr v) t))
491491

492-
(define (vinfo:capt v) (< 0 (logand (caddr v) 1)))
493-
(define (vinfo:asgn v) (< 0 (logand (caddr v) 2)))
494-
(define (vinfo:never-undef v) (< 0 (logand (caddr v) 4)))
495-
(define (vinfo:read v) (< 0 (logand (caddr v) 8)))
496-
(define (vinfo:sa v) (< 0 (logand (caddr v) 16)))
497-
(define (vinfo:nospecialize v) (< 0 (logand (caddr v) 128)))
492+
(define (test-bit x b) (< 0 (logand x b)))
493+
(define (vinfo:capt v) (test-bit (caddr v) 1))
494+
(define (vinfo:asgn v) (test-bit (caddr v) 2))
495+
(define (vinfo:never-undef v) (test-bit (caddr v) 4))
496+
(define (vinfo:read v) (test-bit (caddr v) 8))
497+
(define (vinfo:sa v) (test-bit (caddr v) 16))
498+
(define (vinfo:nospecialize v) (test-bit (caddr v) 128))
498499
(define (set-bit x b val) (if val (logior x b) (logand x (lognot b))))
499500
;; record whether var is captured
500501
(define (vinfo:set-capt! v c) (set-car! (cddr v) (set-bit (caddr v) 1 c)))

src/builtins.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,13 +1537,18 @@ JL_CALLABLE(jl_f_declare_global)
15371537

15381538
JL_CALLABLE(jl_f_declare_const)
15391539
{
1540-
JL_NARGS(declare_const, 2, 3);
1540+
JL_NARGS(declare_const, 2, 4);
15411541
JL_TYPECHK(declare_const, module, args[0]);
15421542
if (nargs == 3)
15431543
JL_TYPECHK(declare_const, symbol, args[1]);
15441544
jl_binding_t *b = jl_get_module_binding((jl_module_t *)args[0], (jl_sym_t *)args[1], 1);
1545-
jl_value_t *val = nargs == 3 ? args[2] : NULL;
1546-
jl_declare_constant_val(b, (jl_module_t *)args[0], (jl_sym_t *)args[1], val);
1545+
jl_value_t *val = nargs >= 3 ? args[2] : NULL;
1546+
uint8_t flags = 0;
1547+
if (nargs == 4) {
1548+
JL_TYPECHK(declare_const, uint8, args[3]);
1549+
flags = jl_unbox_uint8(args[3]);
1550+
}
1551+
jl_declare_constant_val(b, (jl_module_t *)args[0], (jl_sym_t *)args[1], val, flags);
15471552
return nargs > 2 ? args[2] : jl_nothing;
15481553
}
15491554

@@ -1569,14 +1574,14 @@ JL_CALLABLE(jl_f__import)
15691574
}
15701575
else if (nargs == 5) {
15711576
JL_TYPECHK(_import, symbol, args[3]);
1572-
JL_TYPECHK(_import, bool, args[4]);
1577+
JL_TYPECHK(_import, uint8, args[4]);
15731578
jl_module_import(jl_current_task, (jl_module_t *)args[0], (jl_module_t *)args[1],
1574-
(jl_sym_t *)args[2], (jl_sym_t *)args[3], args[4] == jl_true);
1579+
(jl_sym_t *)args[2], (jl_sym_t *)args[3], jl_unbox_uint8(args[4]));
15751580
}
15761581
return jl_nothing;
15771582
}
15781583

1579-
// _using(to::Module, from::Module)
1584+
// _using(to::Module, from::Module, [flags::UInt8=0])
15801585
JL_CALLABLE(jl_f__using)
15811586
{
15821587
JL_NARGS(_using, 2, 3);

src/gf.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4454,7 +4454,7 @@ jl_value_t *jl_new_generic_function_with_supertype(jl_sym_t *name, jl_module_t *
44544454
JL_GC_PUSH1(&ftype);
44554455
ftype->name->singletonname = name;
44564456
jl_gc_wb(ftype->name, name);
4457-
jl_declare_constant_val3(NULL, module, tname, (jl_value_t*)ftype, PARTITION_KIND_CONST, new_world);
4457+
jl_declare_constant_val3(NULL, module, tname, (jl_value_t*)ftype, PARTITION_KIND_CONST, new_world, 0);
44584458
jl_value_t *f = jl_new_struct(ftype);
44594459
ftype->instance = f;
44604460
jl_gc_wb(ftype, f);

0 commit comments

Comments
 (0)