Skip to content

Commit

Permalink
Improve inference for string operations (JuliaLang#44500)
Browse files Browse the repository at this point in the history
This reduces invalidations when loading packages that define new
AbstractString types.
  • Loading branch information
timholy committed Mar 16, 2022
1 parent cc60657 commit 515a242
Show file tree
Hide file tree
Showing 12 changed files with 58 additions and 28 deletions.
4 changes: 3 additions & 1 deletion base/binaryplatforms.jl
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ const libstdcxx_version_mapping = Dict{String,String}(
Parses a string platform triplet back into a `Platform` object.
"""
function Base.parse(::Type{Platform}, triplet::AbstractString; validate_strict::Bool = false)
function Base.parse(::Type{Platform}, triplet::String; validate_strict::Bool = false)
# Helper function to collapse dictionary of mappings down into a regex of
# named capture groups joined by "|" operators
c(mapping) = string("(",join(["(?<$k>$v)" for (k, v) in mapping], "|"), ")")
Expand Down Expand Up @@ -751,6 +751,8 @@ function Base.parse(::Type{Platform}, triplet::AbstractString; validate_strict::
end
throw(ArgumentError("Platform `$(triplet)` is not an officially supported platform"))
end
Base.parse(::Type{Platform}, triplet::AbstractString; kwargs...) =
parse(Platform, convert(String, triplet)::String; kwargs...)

function Base.tryparse(::Type{Platform}, triplet::AbstractString)
try
Expand Down
10 changes: 5 additions & 5 deletions base/io.jl
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ wait_close(io::AbstractPipe) = (wait_close(pipe_writer(io)::IO); wait_close(pipe

# Exception-safe wrappers (io = open(); try f(io) finally close(io))

write(filename::AbstractString, a1, args...) = open(io->write(io, a1, args...), filename, "w")
write(filename::AbstractString, a1, args...) = open(io->write(io, a1, args...), convert(String, filename)::String, "w")

"""
read(filename::AbstractString, args...)
Expand All @@ -457,9 +457,9 @@ Open a file and read its contents. `args` is passed to `read`: this is equivalen
Read the entire contents of a file as a string.
"""
read(filename::AbstractString, args...) = open(io->read(io, args...), filename)
read(filename::AbstractString, args...) = open(io->read(io, args...), convert(String, filename)::String)

read(filename::AbstractString, ::Type{T}) where {T} = open(io->read(io, T), filename)
read(filename::AbstractString, ::Type{T}) where {T} = open(io->read(io, T), convert(String, filename)::String)

"""
read!(stream::IO, array::AbstractArray)
Expand All @@ -469,7 +469,7 @@ Read binary data from an I/O stream or file, filling in `array`.
"""
function read! end

read!(filename::AbstractString, a) = open(io->read!(io, a), filename)
read!(filename::AbstractString, a) = open(io->read!(io, a), convert(String, filename)::String)

"""
readuntil(stream::IO, delim; keep::Bool = false)
Expand All @@ -496,7 +496,7 @@ julia> readuntil("my_file.txt", '.', keep = true)
julia> rm("my_file.txt")
```
"""
readuntil(filename::AbstractString, args...; kw...) = open(io->readuntil(io, args...; kw...), filename)
readuntil(filename::AbstractString, args...; kw...) = open(io->readuntil(io, args...; kw...), convert(String, filename)::String)

"""
readline(io::IO=stdin; keep::Bool=false)
Expand Down
3 changes: 2 additions & 1 deletion base/iostream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ safe multi-threaded access.
!!! compat "Julia 1.5"
The `lock` argument is available as of Julia 1.5.
"""
function open(fname::AbstractString; lock = true,
function open(fname::String; lock = true,
read :: Union{Bool,Nothing} = nothing,
write :: Union{Bool,Nothing} = nothing,
create :: Union{Bool,Nothing} = nothing,
Expand All @@ -299,6 +299,7 @@ function open(fname::AbstractString; lock = true,
end
return s
end
open(fname::AbstractString; kwargs...) = open(convert(String, fname)::String; kwargs...)

"""
open(filename::AbstractString, [mode::AbstractString]; lock = true) -> IOStream
Expand Down
4 changes: 2 additions & 2 deletions base/logging.jl
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ end
msg = try
"Exception while generating log record in module $_module at $filepath:$line"
catch ex
"Exception handling log message: $ex"
LazyString("Exception handling log message: ", ex)
end
bt = real ? catch_backtrace() : backtrace()
handle_message(
Expand Down Expand Up @@ -674,7 +674,7 @@ function handle_message(logger::SimpleLogger, level::LogLevel, message, _module,
end
iob = IOContext(buf, stream)
levelstr = level == Warn ? "Warning" : string(level)
msglines = eachsplit(chomp(string(message)::String), '\n')
msglines = eachsplit(chomp(convert(String, string(message))::String), '\n')
msg1, rest = Iterators.peel(msglines)
println(iob, "", levelstr, ": ", msg1)
for msg in rest
Expand Down
25 changes: 13 additions & 12 deletions base/process.jl
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ function _uv_hook_close(proc::Process)
nothing
end

const SpawnIOs = Vector{Any} # convenience name for readability
const SpawnIO = Union{IO, RawFD, OS_HANDLE}
const SpawnIOs = Vector{SpawnIO} # convenience name for readability

function as_cpumask(cpus::Vector{UInt16})
n = max(Int(maximum(cpus)), Int(ccall(:uv_cpumask_size, Cint, ())))
Expand Down Expand Up @@ -129,7 +130,7 @@ end
return pp
end

_spawn(cmds::AbstractCmd) = _spawn(cmds, Any[])
_spawn(cmds::AbstractCmd) = _spawn(cmds, SpawnIO[])

# optimization: we can spawn `Cmd` directly without allocating the ProcessChain
function _spawn(cmd::Cmd, stdios::SpawnIOs)
Expand Down Expand Up @@ -213,7 +214,7 @@ end
# open the child end of each element of `stdios`, and initialize the parent end
function setup_stdios(f, stdios::SpawnIOs)
nstdio = length(stdios)
open_io = Vector{Any}(undef, nstdio)
open_io = SpawnIOs(undef, nstdio)
close_io = falses(nstdio)
try
for i in 1:nstdio
Expand Down Expand Up @@ -324,19 +325,19 @@ close_stdio(stdio) = close(stdio)
# - An Filesystem.File or IOStream object to redirect the output to
# - A FileRedirect, containing a string specifying a filename to be opened for the child

spawn_opts_swallow(stdios::StdIOSet) = Any[stdios...]
spawn_opts_inherit(stdios::StdIOSet) = Any[stdios...]
spawn_opts_swallow(stdios::StdIOSet) = SpawnIO[stdios...]
spawn_opts_inherit(stdios::StdIOSet) = SpawnIO[stdios...]
spawn_opts_swallow(in::Redirectable=devnull, out::Redirectable=devnull, err::Redirectable=devnull) =
Any[in, out, err]
SpawnIO[in, out, err]
# pass original descriptors to child processes by default, because we might
# have already exhausted and closed the libuv object for our standard streams.
# ref issue #8529
spawn_opts_inherit(in::Redirectable=RawFD(0), out::Redirectable=RawFD(1), err::Redirectable=RawFD(2)) =
Any[in, out, err]
SpawnIO[in, out, err]

function eachline(cmd::AbstractCmd; keep::Bool=false)
out = PipeEndpoint()
processes = _spawn(cmd, Any[devnull, out, stderr])
processes = _spawn(cmd, SpawnIO[devnull, out, stderr])
# if the user consumes all the data, also check process exit status for success
ondone = () -> (success(processes) || pipeline_error(processes); nothing)
return EachLine(out, keep=keep, ondone=ondone)::EachLine
Expand Down Expand Up @@ -384,20 +385,20 @@ function open(cmds::AbstractCmd, stdio::Redirectable=devnull; write::Bool=false,
stdio === devnull || throw(ArgumentError("no stream can be specified for `stdio` in read-write mode"))
in = PipeEndpoint()
out = PipeEndpoint()
processes = _spawn(cmds, Any[in, out, stderr])
processes = _spawn(cmds, SpawnIO[in, out, stderr])
processes.in = in
processes.out = out
elseif read
out = PipeEndpoint()
processes = _spawn(cmds, Any[stdio, out, stderr])
processes = _spawn(cmds, SpawnIO[stdio, out, stderr])
processes.out = out
elseif write
in = PipeEndpoint()
processes = _spawn(cmds, Any[in, stdio, stderr])
processes = _spawn(cmds, SpawnIO[in, stdio, stderr])
processes.in = in
else
stdio === devnull || throw(ArgumentError("no stream can be specified for `stdio` in no-access mode"))
processes = _spawn(cmds, Any[devnull, devnull, stderr])
processes = _spawn(cmds, SpawnIO[devnull, devnull, stderr])
end
return processes
end
Expand Down
4 changes: 2 additions & 2 deletions base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1921,7 +1921,7 @@ function show_unquoted(io::IO, ex::Expr, indent::Int, prec::Int, quote_level::In
na = length(func_args)
if (na == 2 || (na > 2 && isa(func, Symbol) && func in (:+, :++, :*)) || (na == 3 && func === :(:))) &&
all(a -> !isa(a, Expr) || a.head !== :..., func_args)
sep = func === :(:) ? "$func" : " $func "
sep = func === :(:) ? "$func" : " " * convert(String, string(func))::String * " " # if func::Any, avoid string interpolation (invalidation)

if func_prec <= prec
show_enclosed_list(io, '(', func_args, sep, ')', indent, func_prec, quote_level, true)
Expand Down Expand Up @@ -2291,7 +2291,7 @@ function show_unquoted(io::IO, ex::Expr, indent::Int, prec::Int, quote_level::In
elseif head === :meta && nargs == 1 && args[1] === :pop_loc
print(io, "# meta: pop location")
elseif head === :meta && nargs == 2 && args[1] === :pop_loc
print(io, "# meta: pop locations ($(args[2]))")
print(io, "# meta: pop locations ($(args[2]::Int))")
# print anything else as "Expr(head, args...)"
else
unhandled = true
Expand Down
13 changes: 12 additions & 1 deletion base/strings/substring.jl
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ convert(::Type{SubString{S}}, s::AbstractString) where {S<:AbstractString} =
SubString(convert(S, s))
convert(::Type{T}, s::T) where {T<:SubString} = s

# Regex match allows only Union{String, SubString{String}} so define conversion to this type
convert(::Type{Union{String, SubString{String}}}, s::String) = s
convert(::Type{Union{String, SubString{String}}}, s::SubString{String}) = s
convert(::Type{Union{String, SubString{String}}}, s::AbstractString) = convert(String, s)

function String(s::SubString{String})
parent = s.string
copy = GC.@preserve parent unsafe_string(pointer(parent, s.offset+1), s.ncodeunits)
Expand Down Expand Up @@ -229,7 +234,13 @@ function string(a::Union{Char, String, SubString{String}, Symbol}...)
out = _string_n(n)
offs = 1
for v in a
offs += __unsafe_string!(out, v, offs)
if v isa Char
offs += __unsafe_string!(out, v, offs)
elseif v isa String || v isa SubString{String}
offs += __unsafe_string!(out, v, offs)
else
offs += __unsafe_string!(out, v::Symbol, offs)
end
end
return out
end
Expand Down
2 changes: 1 addition & 1 deletion stdlib/Logging/src/ConsoleLogger.jl
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ function handle_message(logger::ConsoleLogger, level::LogLevel, message, _module

# Generate a text representation of the message and all key value pairs,
# split into lines.
msglines = [(indent=0, msg=l) for l in split(chomp(string(message)::String), '\n')]
msglines = [(indent=0, msg=l) for l in split(chomp(convert(String, string(message))::String), '\n')]
stream = logger.stream
if !isopen(stream)
stream = stderr
Expand Down
2 changes: 1 addition & 1 deletion stdlib/REPL/src/LineEdit.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1646,7 +1646,7 @@ end
throw_eager_redirection_cycle(key::Union{Char, String}) =
error("Eager redirection cycle detected for key ", repr(key))
throw_could_not_find_redirected_value(key::Union{Char, String}) =
error("Could not find redirected value ", repl(key))
error("Could not find redirected value ", repr(key))

function keymap_unify(keymaps)
ret = Dict{Char,Any}()
Expand Down
2 changes: 1 addition & 1 deletion stdlib/REPL/src/TerminalMenus/AbstractMenu.jl
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ function request(term::REPL.Terminals.TTYTerminal, m::AbstractMenu; cursor::Unio
REPL.Terminals.raw!(term, true)
true
catch err
suppress_output || @warn("TerminalMenus: Unable to enter raw mode: $err")
suppress_output || @warn "TerminalMenus: Unable to enter raw mode: " exception=(err, catch_backtrace())
false
end
# hide the cursor
Expand Down
2 changes: 1 addition & 1 deletion stdlib/p7zip_jll/src/p7zip_jll.jl
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ else
const pathsep = ':'
end

function adjust_ENV!(env::Dict, PATH::String, LIBPATH::String, adjust_PATH::Bool, adjust_LIBPATH::Bool)
function adjust_ENV!(env::Dict{keytype(Base.EnvDict),valtype(Base.EnvDict)}, PATH::String, LIBPATH::String, adjust_PATH::Bool, adjust_LIBPATH::Bool)
if adjust_LIBPATH
LIBPATH_base = get(env, LIBPATH_env, expanduser(LIBPATH_default))
if !isempty(LIBPATH_base)
Expand Down
15 changes: 15 additions & 0 deletions test/corelogging.jl
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,21 @@ end
@test_throws MethodError @macrocall(@error)
end

@testset "Any type" begin
@test_logs (:info, sum) @info sum
# TODO: make this work (here we want `@test_logs` to fail)
# @test_fails @test_logs (:info, "sum") @info sum # `sum` works, `"sum"` does not

# check that the message delivered to the user works
mktempdir() do dir
path_stdout = joinpath(dir, "stdout.txt")
path_stderr = joinpath(dir, "stderr.txt")
redirect_stdio(stdout=path_stdout, stderr=path_stderr) do
@info sum
end
@test occursin("Info: sum", read(path_stderr, String))
end
end

#-------------------------------------------------------------------------------
# Early log level filtering
Expand Down

0 comments on commit 515a242

Please sign in to comment.