Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 25 additions & 23 deletions src/analysis/occurrence-analysis.jl
Original file line number Diff line number Diff line change
Expand Up @@ -97,22 +97,21 @@ function compute_binding_occurrences(

for (i, binfo) = enumerate(ctx3.bindings.info)
binfo.is_internal && continue
if binfo.kind === :argument
push!(get!(Vector{Int}, same_arg_bindings, Symbol(binfo.name)), i)
if is_generated
# In `@generated` functions, type parameters become actual
# arguments. Include them in location-based merging so they
# get unified with their `:static_parameter` counterparts.
lockey = (Symbol(binfo.name), JS.source_location(JL.binding_ex(ctx3, binfo.id))...)
push!(get!(Vector{Int}, same_location_bindings, lockey), i)
if binfo.kind === :global
include_global_bindings || continue
else
if binfo.kind === :argument
push!(get!(Vector{Int}, same_arg_bindings, Symbol(binfo.name)), i)
end
elseif binfo.kind === :static_parameter || binfo.kind === :local
# Include arguments in location-based merging to unify them with
# `:local` bindings at the same location. This is needed for:
# - `@generated` functions: type parameters become actual arguments
# that must be unified with their `:static_parameter` counterparts.
# - Keyword arguments with dependent defaults: JuliaLowering's
# `scope_nest` creates `:local` bindings in `let` blocks that
# must be unified with the `:argument` binding in the body method.
lockey = (Symbol(binfo.name), JS.source_location(JL.binding_ex(ctx3, binfo.id))...)
push!(get!(Vector{Int}, same_location_bindings, lockey), i)
elseif binfo.kind === :global
include_global_bindings || continue
else
error(lazy"Unknown binding kind: $(binfo.kind)")
end
occurrences[binfo] = Set{BindingOccurrence{Tree3}}()
end
Expand Down Expand Up @@ -157,14 +156,11 @@ function compute_binding_occurrences(
# ```julia
# hasmatch(x::RegexMatch, y::Bool=isempty(x.matches)) = y
# ```
# N.B. This needs to be done separately from `same_location_bindings`.
# This is because if argument lists were also aggregated by "name & location" key,
# then even when `x` is truly unused, the usage in methods that fill default parameters
# and call the full-argument list method would be aggregated, causing us to miss reports
# in such cases, e.g.
# ```julia
# hasmatch(x::RegexMatch, y::Bool=false) = nothing
# ```
# Note: argument bindings are included in `same_location_bindings` above to bridge
# `:argument` and `:local` bindings for keyword arguments with dependent defaults.
# This is safe because `compute_binding_occurrences!` skips both `:argument` and
# `:local` bindings in self/kwsorter calls, preventing internal call machinery from
# being counted as usage.
for (_, idxs) in same_arg_bindings
length(idxs) == 1 && continue
newoccurrences = union!((occurrences[ctx3.bindings.info[idx]] for idx in idxs)...)
Expand Down Expand Up @@ -316,8 +312,14 @@ function compute_binding_occurrences!(
if skip_arguments
for i = nc:-1:2 # reversed since we use `pop!`
argⱼ = st[i]
if JS.kind(argⱼ) === JS.K"BindingId" && JL.get_binding(ctx3, argⱼ).kind === :argument
continue # skip this argument
if JS.kind(argⱼ) === JS.K"BindingId"
bkind = JL.get_binding(ctx3, argⱼ).kind
# Skip both `:argument` and `:local` bindings.
# `:local` bindings appear in kwsorter calls when
# `scope_nest` is used for dependent keyword defaults.
if bkind === :argument || bkind === :local
continue
end
end
push!(stack, st[i])
end
Expand Down
21 changes: 21 additions & 0 deletions src/diagnostic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,21 @@ function is_kwarg_constraining_used_sparam(
return false
end

function has_matching_argument_binding(
binding_occurrences::Dict{JL.BindingInfo,Set{BindingOccurrence{Tree3}}},
name::String, range::Range,
fi::FileInfo, ctx3::JL.VariableAnalysisContext
) where Tree3<:JS.SyntaxTree
for (binfo2, _) in binding_occurrences
binfo2.kind === :argument || continue
binfo2.name == name || continue
provs2 = JS.flattened_provenance(JL.binding_ex(ctx3, binfo2.id))
is_from_user_ast(provs2) || continue
jsobj_to_range(last(provs2), fi) == range && return true
end
return false
end

function analyze_unused_bindings!(
diagnostics::Vector{Diagnostic}, fi::FileInfo, st0::JS.SyntaxTree, ctx3::JL.VariableAnalysisContext,
binding_occurrences::Dict{JL.BindingInfo,Set{BindingOccurrence{Tree3}}},
Expand Down Expand Up @@ -666,6 +681,12 @@ function analyze_unused_bindings!(
range = jsobj_to_range(prov, fi)
key = LoweringDiagnosticKey(range, bk, bn)
key in reported ? continue : push!(reported, key)
if bk === :local && has_matching_argument_binding(binding_occurrences, bn, range, fi, ctx3)
# When `:argument` and `:local` bindings are merged at the same
# location (keyword dependent defaults), only the `:argument`
# diagnostic should be reported.
continue
end
if bk === :argument
message = "Unused argument `$bn`"
code = LOWERING_UNUSED_ARGUMENT_CODE
Expand Down
19 changes: 19 additions & 0 deletions test/test_lowering_diagnostic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,25 @@ length_utf16(s::AbstractString) = sum(c::Char -> codepoint(c) < 0x10000 ? 1 : 2,
""")
@test isempty(diagnostics)
end

# aviatesk/JETLS.jl#592
let diagnostics = get_lowered_diagnostics("""
function group(
by,
f,
itr;
T::Type = eltype(itr),
By::Type = only(Base.return_types(by, (T,))),
F::Type = only(Base.return_types(f, (T,))),
)::Dict{By, Vector{F}}
return foldl(itr; init = Dict{By, Vector{F}}()) do acc, x
push!(get!(acc, by(x), F[]), f(x))
return acc
end
end
""")
@test isempty(diagnostics)
end
end

@testset "module splitter" begin
Expand Down
Loading