From e549b7f46b4b362b758be623d734031f5b21f7a4 Mon Sep 17 00:00:00 2001 From: Neven Sajko Date: Sun, 11 Aug 2024 16:25:06 +0200 Subject: [PATCH] constrain the path argument of `include` functions to `AbstractString` 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. --- base/loading.jl | 4 ++-- src/jlfrontend.scm | 4 ++-- test/loading.jl | 11 +++++++++++ test/testhelpers/just_module.jl | 1 + 4 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 test/testhelpers/just_module.jl diff --git a/base/loading.jl b/base/loading.jl index 4dc735f0099d8..e338cc1e48169 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -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...]) diff --git a/src/jlfrontend.scm b/src/jlfrontend.scm index 2c5f42eda5ce8..463e39c41d00a 100644 --- a/src/jlfrontend.scm +++ b/src/jlfrontend.scm @@ -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))))) diff --git a/test/loading.jl b/test/loading.jl index 8310cb03c410b..d78a5b602be75 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -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) diff --git a/test/testhelpers/just_module.jl b/test/testhelpers/just_module.jl new file mode 100644 index 0000000000000..71bd87e660eae --- /dev/null +++ b/test/testhelpers/just_module.jl @@ -0,0 +1 @@ +@__MODULE__