Skip to content

Commit f98eee6

Browse files
committed
Take 2 on use of global x in value position
Disallows "use" of the value of `global x`, except in tail position in top level thunks - in those cases, return `nothing` so that `global x` can be used in the value position of a top level thunk. This is normally harmless as one cannot observe this value, except in special circumstances - namely the return value of `eval()` (and things which call eval, such as `include()`). While we're thinking about this, also disallow a bare `local x` in a top level thunk because this cannot have useful side effects and is just confusing when it occurs outside a block construct. (This is not currently disallowed for `local` arising from macro expansions because it's not an obvious user error in that case. It could possibly arise as a valid macro expansion of a trivial case, for some macros?)
1 parent 6d2601a commit f98eee6

File tree

7 files changed

+85
-14
lines changed

7 files changed

+85
-14
lines changed

src/desugaring.jl

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2173,9 +2173,6 @@ function expand_decls(ctx, ex)
21732173
throw(LoweringError(ex, "invalid syntax in variable declaration"))
21742174
end
21752175
end
2176-
if kind(stmts[end]) in KSet"local global"
2177-
push!(stmts, @ast ctx ex "nothing"::K"core")
2178-
end
21792176
makenode(ctx, ex, K"block", stmts)
21802177
end
21812178

src/kinds.jl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ function _register_kinds()
117117
# (That is, it's removable in the same sense as
118118
# `@assume_effects :removable`.)
119119
"removable"
120+
# Variable type declaration; `x::T = rhs` will be temporarily
121+
# desugared to include `(decl x T)`
120122
"decl"
121123
# [K"captured_local" index]
122124
# A local variable captured into a global method. Contains the

src/linear_ir.jl

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -792,12 +792,20 @@ function compile(ctx::LinearIRContext, ex, needs_value, in_tail_pos)
792792
emit(ctx, ex)
793793
nothing
794794
elseif k == K"global"
795-
if needs_value
796-
throw(LoweringError(ex, "misplaced global declaration in value position"))
797-
end
798795
emit(ctx, ex)
799796
ctx.is_toplevel_thunk && emit_latestworld(ctx, ex)
800-
nothing
797+
if needs_value
798+
if in_tail_pos && ctx.is_toplevel_thunk
799+
# Permit "statement-like" globals at top level but potentially
800+
# inside blocks.
801+
compile(ctx, nothing_(ctx, ex), needs_value, in_tail_pos)
802+
else
803+
throw(LoweringError(ex,
804+
"global declaration doesn't read the variable and can't return a value"))
805+
end
806+
else
807+
nothing
808+
end
801809
elseif k == K"meta"
802810
emit(ctx, ex)
803811
if needs_value

src/macro_expansion.jl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,11 @@ function expand_forms_1(ctx::MacroExpansionContext, ex::SyntaxTree)
461461
end
462462

463463
@fzone "JL: macroexpand" function expand_forms_1(mod::Module, ex::SyntaxTree, expr_compat_mode::Bool, macro_world::UInt)
464+
if kind(ex) == K"local"
465+
# This error assumes we're expanding the body of a top level thunk but
466+
# we might want to make that more explicit in the pass system.
467+
throw(LoweringError(ex, "local declarations have no effect outside a scope"))
468+
end
464469
graph = ensure_attributes(syntax_graph(ex),
465470
var_id=IdTag,
466471
scope_layer=LayerId,

src/scope_analysis.jl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,10 @@ function _resolve_scopes(ctx, ex::SyntaxTree)
427427
# elseif k == K"global"
428428
# ex
429429
elseif k == K"local"
430-
makeleaf(ctx, ex, K"TOMBSTONE")
430+
# Local declarations have a value of `nothing` according to flisp
431+
# lowering.
432+
# TODO: Should local decls be disallowed in value position?
433+
@ast ctx ex "nothing"::K"core"
431434
elseif k == K"decl"
432435
ex_out = mapchildren(e->_resolve_scopes(ctx, e), ctx, ex)
433436
name = ex_out[1]

test/decls.jl

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@ end
1111

1212
# In value position, yield the right hand side, not `x`
1313
@test JuliaLowering.include_string(test_mod, """
14-
local x::Int = 1.0
14+
begin
15+
local x::Int = 1.0
16+
end
1517
""") === 1.0
1618

17-
# Decsl in value position without assignment return nothing
19+
# Global decl in value position without assignment returns nothing
1820
@test JuliaLowering.include_string(test_mod, "global x_no_assign") === nothing
19-
@test JuliaLowering.include_string(test_mod, "local x_no_assign") === nothing
2021

2122
# Unadorned declarations
2223
@test JuliaLowering.include_string(test_mod, """

test/decls_ir.jl

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
########################################
22
# Local declaration with type
3-
local x::T = 1
3+
begin
4+
local x::T = 1
5+
end
46
#---------------------
57
1 (newvar slot₁/x)
68
2 1
@@ -18,20 +20,73 @@ local x::T = 1
1820
14 (return %₂)
1921

2022
########################################
21-
# Local declaration in value position without assignment
23+
# Error: Local declarations outside a scope are disallowed
24+
# See https://github.com/JuliaLang/julia/issues/57483
2225
local x
2326
#---------------------
27+
LoweringError:
28+
local x
29+
└─────┘ ── local declarations have no effect outside a scope
30+
31+
########################################
32+
# Local declaration allowed in tail position
33+
begin
34+
local x
35+
end
36+
#---------------------
2437
1 (newvar slot₁/x)
2538
2 (return core.nothing)
2639

2740
########################################
28-
# Global declaration in value position without assignment
41+
# Local declaration allowed in value position
42+
# TODO: This may be a bug in flisp lowering - should we reconsider this?
43+
let
44+
y = local x
45+
end
46+
#---------------------
47+
1 (newvar slot₁/x)
48+
2 core.nothing
49+
3 (= slot₂/y %₂)
50+
4 (return %₂)
51+
52+
########################################
53+
# Global declaration allowed in tail position
2954
global x
3055
#---------------------
3156
1 (global TestMod.x)
3257
2 latestworld
3358
3 (return core.nothing)
3459

60+
########################################
61+
# Global declaration allowed in tail position, nested
62+
begin
63+
global x
64+
end
65+
#---------------------
66+
1 (global TestMod.x)
67+
2 latestworld
68+
3 (return core.nothing)
69+
70+
########################################
71+
# Error: Global declaration not allowed in tail position in functions
72+
function f()
73+
global x
74+
end
75+
#---------------------
76+
LoweringError:
77+
function f()
78+
global x
79+
# ╙ ── global declaration doesn't read the variable and can't return a value
80+
end
81+
82+
########################################
83+
# Error: Global declaration not allowed in value position
84+
y = global x
85+
#---------------------
86+
LoweringError:
87+
y = global x
88+
# ╙ ── global declaration doesn't read the variable and can't return a value
89+
3590
########################################
3691
# const
3792
const xx = 10

0 commit comments

Comments
 (0)