Skip to content

Commit

Permalink
constrain the path argument of include functions to AbstractString
Browse files Browse the repository at this point in the history
Each `Module` defined with `module` automatically gets an `include`
function with two methods. Each of those two methods takes a file path
as its last argument. Even though the path argument is unconstrained by
dispatch, it's documented as constrained with `::AbstractString`:

https://docs.julialang.org/en/v1.11-dev/base/base/#include

Furthermore, I think that any invocation of `include` with a
non-`AbstractString` path will necessarily throw a `MethodError`
eventually. Thus this change should be harmless.

Adding the type constraint to the path argument is an improvement
because any possible exception would be thrown earlier than before.

Apart from modules defined with `module`, the same issue is present
with the anonymous modules created by `evalfile`, which is also
addressed.

Sidenote: `evalfile` seems to be completely untested apart from the
test added here.
  • Loading branch information
nsajko committed Aug 11, 2024
1 parent 2e1235e commit e549b7f
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 4 deletions.
4 changes: 2 additions & 2 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2715,8 +2715,8 @@ function evalfile(path::AbstractString, args::Vector{String}=String[])
Expr(:toplevel,
:(const ARGS = $args),
:(eval(x) = $(Expr(:core, :eval))(__anon__, x)),
:(include(x) = $(Expr(:top, :include))(__anon__, x)),
:(include(mapexpr::Function, x) = $(Expr(:top, :include))(mapexpr, __anon__, x)),
:(include(x::AbstractString) = $(Expr(:top, :include))(__anon__, x)),
:(include(mapexpr::Function, x::AbstractString) = $(Expr(:top, :include))(mapexpr, __anon__, x)),
:(include($path))))
end
evalfile(path::AbstractString, args::Vector) = evalfile(path, String[args...])
Expand Down
4 changes: 2 additions & 2 deletions src/jlfrontend.scm
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,11 @@
(block
,@loc
(call (core eval) ,name ,x)))
(= (call include ,x)
(= (call include (:: ,x (top AbstractString)))
(block
,@loc
(call (core _call_latest) (top include) ,name ,x)))
(= (call include (:: ,mex (top Function)) ,x)
(= (call include (:: ,mex (top Function)) (:: ,x (top AbstractString)))
(block
,@loc
(call (core _call_latest) (top include) ,mex ,name ,x)))))
Expand Down
11 changes: 11 additions & 0 deletions test/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,17 @@ import .Foo28190.Libdl; import Libdl
end
end

@testset "`::AbstractString` constraint on the path argument to `include`" begin
for m (NotPkgModule, evalfile("testhelpers/just_module.jl"))
let i = m.include
@test !applicable(i, (nothing,))
@test !applicable(i, (identity, nothing,))
@test !hasmethod(i, Tuple{Nothing})
@test !hasmethod(i, Tuple{Function,Nothing})
end
end
end

@testset "`Base.project_names` and friends" begin
# Some functions in Pkg assumes that these tuples have the same length
n = length(Base.project_names)
Expand Down
1 change: 1 addition & 0 deletions test/testhelpers/just_module.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@__MODULE__

0 comments on commit e549b7f

Please sign in to comment.