From 4de760b4156a08b746c38d9b1d6601162aa95a98 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Fri, 4 Apr 2025 10:36:24 -0400 Subject: [PATCH 1/9] Re-enable tab completion of kwargs for large method tables while testing to ensure that absurdly large method tables don't tank the performance of the REPL --- stdlib/REPL/src/REPLCompletions.jl | 2 +- stdlib/REPL/test/replcompletions.jl | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/stdlib/REPL/src/REPLCompletions.jl b/stdlib/REPL/src/REPLCompletions.jl index d4e33e8ff5136..12aa6c936d553 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -1082,7 +1082,7 @@ function complete_keyword_argument(partial::String, last_idx::Int, context_modul kwargs_flag == 2 && return fail # one of the previous kwargs is invalid methods = Completion[] - complete_methods!(methods, funct, Any[Vararg{Any}], kwargs_ex, shift ? -1 : MAX_METHOD_COMPLETIONS, kwargs_flag == 1) + complete_methods!(methods, funct, Any[Vararg{Any}], kwargs_ex, shift ? -1 : 500, kwargs_flag == 1) # TODO: use args_ex instead of Any[Vararg{Any}] and only provide kwarg completion for # method calls compatible with the current arguments. diff --git a/stdlib/REPL/test/replcompletions.jl b/stdlib/REPL/test/replcompletions.jl index d9aa11cf609dd..b2f5d75c60d26 100644 --- a/stdlib/REPL/test/replcompletions.jl +++ b/stdlib/REPL/test/replcompletions.jl @@ -2490,3 +2490,32 @@ const issue57780 = ["a", "b", "c"] const issue57780_orig = copy(issue57780) test_complete_context("empty!(issue57780).", Main) @test issue57780 == issue57780_orig + +function g54131 end +for i in 1:498 + @eval g54131(::Val{$i}) = i +end +g54131(::Val{499}; kwarg=true) = 499*kwarg +struct F54131; end +Base.getproperty(::F54131, ::Symbol) = Any[cos, sin, g54131][rand(1:3)] +@testset "performance of kwarg completion with large method tables" begin + # The goal here is to simply ensure we aren't hitting catestrophically bad + # behaviors when shift isn't pressed. The difference between good and bad + # is on the order of tens of milliseconds vs tens of seconds; using 1 sec as + # a very rough canary that is hopefully robust even in the noisy CI coalmines + s = "g54131(kwa" + a, b, c = completions(s, lastindex(s), @__MODULE__, #= shift =# false) + @test REPLCompletions.KeywordArgumentCompletion("kwarg") in a + @test (@elapsed completions(s, lastindex(s), @__MODULE__, false)) < 1 + + f = F54131() + s = "f.x(" + a, b, c = completions(s, lastindex(s), @__MODULE__, false) + @test isempty(a) + @test (@elapsed completions(s, lastindex(s), @__MODULE__, false)) < 1 + + s = "f.x(kwa" + a, b, c = completions(s, lastindex(s), @__MODULE__, false) + @test_broken REPLCompletions.KeywordArgumentCompletion("kwarg") in a + @test (@elapsed completions(s, lastindex(s), @__MODULE__, false)) < 1 +end From 58929c28b1f041d205c0163f018c233fa0d5ac28 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Fri, 4 Apr 2025 11:26:01 -0400 Subject: [PATCH 2/9] we get a textcompletion back, not an empty one --- stdlib/REPL/test/replcompletions.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/REPL/test/replcompletions.jl b/stdlib/REPL/test/replcompletions.jl index b2f5d75c60d26..124c55dfcec31 100644 --- a/stdlib/REPL/test/replcompletions.jl +++ b/stdlib/REPL/test/replcompletions.jl @@ -2511,7 +2511,7 @@ Base.getproperty(::F54131, ::Symbol) = Any[cos, sin, g54131][rand(1:3)] f = F54131() s = "f.x(" a, b, c = completions(s, lastindex(s), @__MODULE__, false) - @test isempty(a) + @test only(a) isa REPLCompletions.TextCompletion @test (@elapsed completions(s, lastindex(s), @__MODULE__, false)) < 1 s = "f.x(kwa" From 7fdea0a54a5a0404353f71b83284b539735959b4 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Tue, 8 Apr 2025 10:32:59 -0400 Subject: [PATCH 3/9] fixup: object needs to be global to tab complete --- stdlib/REPL/test/replcompletions.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stdlib/REPL/test/replcompletions.jl b/stdlib/REPL/test/replcompletions.jl index 124c55dfcec31..629f6e70c50a4 100644 --- a/stdlib/REPL/test/replcompletions.jl +++ b/stdlib/REPL/test/replcompletions.jl @@ -2498,6 +2498,7 @@ end g54131(::Val{499}; kwarg=true) = 499*kwarg struct F54131; end Base.getproperty(::F54131, ::Symbol) = Any[cos, sin, g54131][rand(1:3)] +f54131 = F54131() @testset "performance of kwarg completion with large method tables" begin # The goal here is to simply ensure we aren't hitting catestrophically bad # behaviors when shift isn't pressed. The difference between good and bad @@ -2508,13 +2509,12 @@ Base.getproperty(::F54131, ::Symbol) = Any[cos, sin, g54131][rand(1:3)] @test REPLCompletions.KeywordArgumentCompletion("kwarg") in a @test (@elapsed completions(s, lastindex(s), @__MODULE__, false)) < 1 - f = F54131() - s = "f.x(" + s = "f54131.x(" a, b, c = completions(s, lastindex(s), @__MODULE__, false) @test only(a) isa REPLCompletions.TextCompletion @test (@elapsed completions(s, lastindex(s), @__MODULE__, false)) < 1 - s = "f.x(kwa" + s = "f54131.x(kwa" a, b, c = completions(s, lastindex(s), @__MODULE__, false) @test_broken REPLCompletions.KeywordArgumentCompletion("kwarg") in a @test (@elapsed completions(s, lastindex(s), @__MODULE__, false)) < 1 From 664bcdada73fc6230cf356b0bf098d5bdd1c681c Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Tue, 8 Apr 2025 14:01:36 -0400 Subject: [PATCH 4/9] add comment --- stdlib/REPL/src/REPLCompletions.jl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/stdlib/REPL/src/REPLCompletions.jl b/stdlib/REPL/src/REPLCompletions.jl index 12aa6c936d553..47e719b46b5eb 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -1082,6 +1082,11 @@ function complete_keyword_argument(partial::String, last_idx::Int, context_modul kwargs_flag == 2 && return fail # one of the previous kwargs is invalid methods = Completion[] + # limit the number of methods to prevent performance issues while allowing + # completions for nearly all common functions. This is distinct from the other + # uses of MAX_METHOD_COMPLETIONS, which primarily used to limit the number + # of methods to _display_ before showing an alternative message; here we're + # searching a larger number to find a subset that contains matching kwargs complete_methods!(methods, funct, Any[Vararg{Any}], kwargs_ex, shift ? -1 : 500, kwargs_flag == 1) # TODO: use args_ex instead of Any[Vararg{Any}] and only provide kwarg completion for # method calls compatible with the current arguments. From 53e4bbee3cd9907bb87d8db8a1e390d0514daf4a Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Tue, 8 Apr 2025 14:42:33 -0400 Subject: [PATCH 5/9] whitespace --- stdlib/REPL/src/REPLCompletions.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/REPL/src/REPLCompletions.jl b/stdlib/REPL/src/REPLCompletions.jl index 47e719b46b5eb..5057f40249be9 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -1085,7 +1085,7 @@ function complete_keyword_argument(partial::String, last_idx::Int, context_modul # limit the number of methods to prevent performance issues while allowing # completions for nearly all common functions. This is distinct from the other # uses of MAX_METHOD_COMPLETIONS, which primarily used to limit the number - # of methods to _display_ before showing an alternative message; here we're + # of methods to _display_ before showing an alternative message; here we're # searching a larger number to find a subset that contains matching kwargs complete_methods!(methods, funct, Any[Vararg{Any}], kwargs_ex, shift ? -1 : 500, kwargs_flag == 1) # TODO: use args_ex instead of Any[Vararg{Any}] and only provide kwarg completion for From ff10270b4c4b21d19be54747b515adb3f8873e0e Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Fri, 11 Apr 2025 10:02:37 -0400 Subject: [PATCH 6/9] instead of a numeric limit, only allow tab completion for non-abstract functions --- stdlib/REPL/src/REPLCompletions.jl | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/stdlib/REPL/src/REPLCompletions.jl b/stdlib/REPL/src/REPLCompletions.jl index 5057f40249be9..7a2e64344bf76 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -1082,12 +1082,15 @@ function complete_keyword_argument(partial::String, last_idx::Int, context_modul kwargs_flag == 2 && return fail # one of the previous kwargs is invalid methods = Completion[] - # limit the number of methods to prevent performance issues while allowing - # completions for nearly all common functions. This is distinct from the other - # uses of MAX_METHOD_COMPLETIONS, which primarily used to limit the number - # of methods to _display_ before showing an alternative message; here we're - # searching a larger number to find a subset that contains matching kwargs - complete_methods!(methods, funct, Any[Vararg{Any}], kwargs_ex, shift ? -1 : 500, kwargs_flag == 1) + # Limit kwarg completions to cases when function is concretely known; looking up + # matching methods for abstract functions — particularly `Any` or `Function` — can + # take many seconds to run over the thousands of possible methods. Note that + # isabstracttype would return naively return true for common constructor calls + # like Array, but the REPL's introspection here may know their Type{T}. + isabstract(f) = isabstracttype(f) + isabstract(::Type{Type{T}}) where {T} = isabstracttype(T) + isabstract(funct) && return fail + complete_methods!(methods, funct, Any[Vararg{Any}], kwargs_ex, -1, kwargs_flag == 1) # TODO: use args_ex instead of Any[Vararg{Any}] and only provide kwarg completion for # method calls compatible with the current arguments. From c23c9f3436c170aa619caaaec38e44355c8089a9 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Thu, 29 May 2025 10:28:54 -0400 Subject: [PATCH 7/9] Fixup merge; return false, not fail --- stdlib/REPL/src/REPLCompletions.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/REPL/src/REPLCompletions.jl b/stdlib/REPL/src/REPLCompletions.jl index 3c983d45d4e8d..a49e791308880 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -917,7 +917,7 @@ function complete_keyword_argument!(suggestions::Vector{Completion}, # like Array, but the REPL's introspection here may know their Type{T}. isabstract(f) = isabstracttype(f) isabstract(::Type{Type{T}}) where {T} = isabstracttype(T) - isabstract(funct) && return fail + isabstract(funct) && return false complete_methods!(methods, funct, Any[Vararg{Any}], kwargs_ex, -1, arg_pos == :kwargs) # TODO: use args_ex instead of Any[Vararg{Any}] and only provide kwarg completion for # method calls compatible with the current arguments. From 4532cbe388f90807eee7df6af189d74a7158d401 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Thu, 29 May 2025 12:58:46 -0400 Subject: [PATCH 8/9] use `isconcretetype` --- stdlib/REPL/src/REPLCompletions.jl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/stdlib/REPL/src/REPLCompletions.jl b/stdlib/REPL/src/REPLCompletions.jl index a49e791308880..b537038fbd61c 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -915,9 +915,7 @@ function complete_keyword_argument!(suggestions::Vector{Completion}, # take many seconds to run over the thousands of possible methods. Note that # isabstracttype would return naively return true for common constructor calls # like Array, but the REPL's introspection here may know their Type{T}. - isabstract(f) = isabstracttype(f) - isabstract(::Type{Type{T}}) where {T} = isabstracttype(T) - isabstract(funct) && return false + isconcretype(funct) || return false complete_methods!(methods, funct, Any[Vararg{Any}], kwargs_ex, -1, arg_pos == :kwargs) # TODO: use args_ex instead of Any[Vararg{Any}] and only provide kwarg completion for # method calls compatible with the current arguments. From a708a92a425b1f8ec0899b33743cef212369fe1c Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Thu, 29 May 2025 14:21:38 -0400 Subject: [PATCH 9/9] fix spelling --- stdlib/REPL/src/REPLCompletions.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/REPL/src/REPLCompletions.jl b/stdlib/REPL/src/REPLCompletions.jl index b537038fbd61c..31983db72bc88 100644 --- a/stdlib/REPL/src/REPLCompletions.jl +++ b/stdlib/REPL/src/REPLCompletions.jl @@ -915,7 +915,7 @@ function complete_keyword_argument!(suggestions::Vector{Completion}, # take many seconds to run over the thousands of possible methods. Note that # isabstracttype would return naively return true for common constructor calls # like Array, but the REPL's introspection here may know their Type{T}. - isconcretype(funct) || return false + isconcretetype(funct) || return false complete_methods!(methods, funct, Any[Vararg{Any}], kwargs_ex, -1, arg_pos == :kwargs) # TODO: use args_ex instead of Any[Vararg{Any}] and only provide kwarg completion for # method calls compatible with the current arguments.