diff --git a/src/analysis/occurrence-analysis.jl b/src/analysis/occurrence-analysis.jl index 51244079..6f0fb30d 100644 --- a/src/analysis/occurrence-analysis.jl +++ b/src/analysis/occurrence-analysis.jl @@ -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 @@ -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)...) @@ -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 diff --git a/src/diagnostic.jl b/src/diagnostic.jl index 296dff55..3e56573a 100644 --- a/src/diagnostic.jl +++ b/src/diagnostic.jl @@ -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}}}, @@ -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 diff --git a/test/test_lowering_diagnostic.jl b/test/test_lowering_diagnostic.jl index c05f90bd..3982de8c 100644 --- a/test/test_lowering_diagnostic.jl +++ b/test/test_lowering_diagnostic.jl @@ -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