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
#55466)

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.

Co-authored-by: Florian <[email protected]>
  • Loading branch information
nsajko and fatteneder authored Sep 24, 2024
1 parent a06a801 commit 060035d
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 @@ -2773,8 +2773,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 060035d

Please sign in to comment.