From 4291e80c0445bd717d60047be5ef001213730c13 Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Mon, 4 Mar 2024 13:42:48 +0100 Subject: [PATCH 1/8] Adding threads --- src/linting/extended_checks.jl | 9 +- test/rai_rules_tests.jl | 178 +++++++++++++++++---------------- 2 files changed, 101 insertions(+), 86 deletions(-) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index fd9e79c..045265e 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -63,8 +63,7 @@ struct Inbounds_Extension <: ExtendedRule end struct Atomic_Extension <: ExtendedRule end struct Ptr_Extension <: ExtendedRule end struct ArrayWithNoType_Extension <: ExtendedRule end - - +struct Threads_Extension <: ExtendedRule end const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule)) @@ -178,3 +177,9 @@ function check(::ArrayWithNoType_Extension, x::EXPR, markers::Dict{Symbol,String contains(markers[:filename], "src/Compiler") || return generic_check(x, "[]", "Need a specific Array type to be provided.") end + +function check(::Threads_Extension, x::EXPR) + msg = "`@threads` should be used with extreme caution." + generic_check(x, "Threads.@threads hole_variable", msg) + generic_check(x, "@threads hole_variable", msg) +end diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index 5c10038..8c82542 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -64,105 +64,30 @@ end "Line 2, column 5: Macro `@cfunction` should not be used.") end - @testset "@lock" begin + @testset "@lock ,@threads" begin source = """ function mark_transaction_as_database_creation!(kv::SpcsKV, transaction_id::String) @lock kv.latch begin push!(kv.create_database_transactions, transaction_id) end + + + Threads.@threads for (e, v) in slots + @test e[] == v + free_slot!(pool, Blob{Nothing}(e)) + end return nothing end """ @test lint_has_error_test(source) @test lint_test(source, "Line 2, column 5: `@lock` should be used with extreme caution") + @test lint_test(source, + "Line 7, column 5: `@threads` should be used with extreme caution.") end - @testset "Locally disabling lint" begin - @testset "lint-disable-lint" begin - @test !lint_has_error_test(""" - function f() - @async 1 + 2 # lint-disable-line - end - """) - @test !lint_has_error_test(""" - function f() - @async 1 + 2 #lint-disable-line - end - """) - - @test !lint_has_error_test(""" - function f() - @async 1 + 2 # lint-disable-line - end - """) - - @test lint_has_error_test(""" - function f() - @async 1 + 2 # lint-disable-line - @async 1 + 3 - end - """) - end - @testset "lint-disable-next-line" begin - @test !lint_has_error_test(""" - function f() - # lint-disable-next-line - @async 1 + 2 - end - """) - @test !lint_has_error_test(""" - function f() - # lint-disable-next-line - @async 1 + 2 - end - """) - - @test !lint_has_error_test(""" - function f() - # lint-disable-next-line - @async 1 + 2 - end - """) - - @test lint_has_error_test(""" - function f() - # lint-disable-next-line - @async 1 + 2 - - @async 1 + 3 - end - """) - @test lint_has_error_test(""" - function f() - @async 1 + 2 - # lint-disable-next-line - - @async 1 + 3 - end - """) - @test lint_has_error_test(""" - function f() - @async 1 + 2 - # lint-disable-next-line - @async 1 + 3 - end - """) - - source = """ - function f() - # lint-disable-next-line - @async 1 + 2 - @async 1 + 3 - end - """ - source_lines = split(source, "\n") - @test convert_offset_to_line_from_lines(46, source_lines) == (3, 4, Symbol("lint-disable-line")) - @test convert_offset_to_line_from_lines(64, source_lines) == (5, 4, nothing) - end - end end @testset "forbidden functions" begin @@ -875,4 +800,89 @@ end @test iszero(StaticLint.run_lint(dir)) end end +end + +@testset "Locally disabling lint" begin + @testset "lint-disable-lint" begin + @test !lint_has_error_test(""" + function f() + @async 1 + 2 # lint-disable-line + end + """) + @test !lint_has_error_test(""" + function f() + @async 1 + 2 #lint-disable-line + end + """) + + @test !lint_has_error_test(""" + function f() + @async 1 + 2 # lint-disable-line + end + """) + + @test lint_has_error_test(""" + function f() + @async 1 + 2 # lint-disable-line + @async 1 + 3 + end + """) + end + @testset "lint-disable-next-line" begin + @test !lint_has_error_test(""" + function f() + # lint-disable-next-line + @async 1 + 2 + end + """) + @test !lint_has_error_test(""" + function f() + # lint-disable-next-line + @async 1 + 2 + end + """) + + @test !lint_has_error_test(""" + function f() + # lint-disable-next-line + @async 1 + 2 + end + """) + + @test lint_has_error_test(""" + function f() + # lint-disable-next-line + @async 1 + 2 + + @async 1 + 3 + end + """) + @test lint_has_error_test(""" + function f() + @async 1 + 2 + # lint-disable-next-line + + @async 1 + 3 + end + """) + @test lint_has_error_test(""" + function f() + @async 1 + 2 + # lint-disable-next-line + @async 1 + 3 + end + """) + + source = """ + function f() + # lint-disable-next-line + @async 1 + 2 + + @async 1 + 3 + end + """ + source_lines = split(source, "\n") + @test convert_offset_to_line_from_lines(46, source_lines) == (3, 4, Symbol("lint-disable-line")) + @test convert_offset_to_line_from_lines(64, source_lines) == (5, 4, nothing) + end end \ No newline at end of file From c241f9db48c417b2eff190a6bf12e540f76d4f90 Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Mon, 4 Mar 2024 16:52:52 +0100 Subject: [PATCH 2/8] Added @generated --- src/linting/extended_checks.jl | 4 ++++ test/rai_rules_tests.jl | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index 045265e..a46a454 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -64,6 +64,7 @@ struct Atomic_Extension <: ExtendedRule end struct Ptr_Extension <: ExtendedRule end struct ArrayWithNoType_Extension <: ExtendedRule end struct Threads_Extension <: ExtendedRule end +struct Generated_Extension <: ExtendedRule end const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule)) @@ -183,3 +184,6 @@ function check(::Threads_Extension, x::EXPR) generic_check(x, "Threads.@threads hole_variable", msg) generic_check(x, "@threads hole_variable", msg) end + +check(::Generated_Extension, x::EXPR) = generic_check(x, "@generated hole_variable") + diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index 8c82542..55cea53 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -64,7 +64,7 @@ end "Line 2, column 5: Macro `@cfunction` should not be used.") end - @testset "@lock ,@threads" begin + @testset "@lock, @threads, @generated" begin source = """ function mark_transaction_as_database_creation!(kv::SpcsKV, transaction_id::String) @lock kv.latch begin @@ -78,12 +78,19 @@ end end return nothing end + + @generated function _empty_vector(::Type{T}) where {T} + vec = T[] + return vec + end """ @test lint_has_error_test(source) @test lint_test(source, "Line 2, column 5: `@lock` should be used with extreme caution") @test lint_test(source, "Line 7, column 5: `@threads` should be used with extreme caution.") + @test lint_test(source, + "Line 14, column 1: `@generated` should be used with extreme caution.") end From 541cc25b59e0030a93faebeea3b044191a768da5 Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Mon, 4 Mar 2024 16:57:28 +0100 Subject: [PATCH 3/8] Adding @sync --- src/linting/extended_checks.jl | 6 ++++++ test/rai_rules_tests.jl | 21 ++++++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index a46a454..494200b 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -65,6 +65,7 @@ struct Ptr_Extension <: ExtendedRule end struct ArrayWithNoType_Extension <: ExtendedRule end struct Threads_Extension <: ExtendedRule end struct Generated_Extension <: ExtendedRule end +struct Sync_Extension <: ExtendedRule end const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule)) @@ -187,3 +188,8 @@ end check(::Generated_Extension, x::EXPR) = generic_check(x, "@generated hole_variable") +function check(::Sync_Extension, x::EXPR) + msg = "`@sync` should be used with extreme caution." + generic_check(x, "@sync hole_variable", msg) + generic_check(x, "Threads.@sync hole_variable", msg) +end diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index 55cea53..e7bb2c4 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -83,6 +83,20 @@ end vec = T[] return vec end + + try + @sync begin + @spawn_pager_bg_task RAI_PagerCore.add_to_cache!(conf, page) + @span_no_threshold "eot_rootmarker_write_to_blob" write_marker_to_cloud( + conf, page, rootinfo + ) + end + catch + Threads.@sync begin + 1 + 2 + end + nothing + end """ @test lint_has_error_test(source) @test lint_test(source, @@ -91,10 +105,11 @@ end "Line 7, column 5: `@threads` should be used with extreme caution.") @test lint_test(source, "Line 14, column 1: `@generated` should be used with extreme caution.") + @test lint_test(source, + "Line 20, column 5: `@sync` should be used with extreme caution.") + @test lint_test(source, + "Line 27, column 5: `@sync` should be used with extreme caution.") end - - - end @testset "forbidden functions" begin From e0c02e7396d57938dbcd99f282bd19693d9f70b4 Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Mon, 4 Mar 2024 17:01:29 +0100 Subject: [PATCH 4/8] Added `remove_page` --- src/linting/extended_checks.jl | 3 +++ test/rai_rules_tests.jl | 10 +++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index 494200b..fbd66e5 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -66,6 +66,7 @@ struct ArrayWithNoType_Extension <: ExtendedRule end struct Threads_Extension <: ExtendedRule end struct Generated_Extension <: ExtendedRule end struct Sync_Extension <: ExtendedRule end +struct RemovePage_Extension <: ExtendedRule end const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule)) @@ -193,3 +194,5 @@ function check(::Sync_Extension, x::EXPR) generic_check(x, "@sync hole_variable", msg) generic_check(x, "Threads.@sync hole_variable", msg) end + +check(::RemovePage_Extension, x::EXPR) = generic_check(x, "remove_page(hole_variable,hole_variable)") diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index e7bb2c4..155a85f 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -362,7 +362,7 @@ end "Line 18, column 5: `wait` should be used with extreme caution.") end - @testset "fetch, @inbounds, Atomic, Ptr" begin + @testset "fetch, @inbounds, Atomic, Ptr, remove_page" begin source = """ function f() fut = Future{Any}() @@ -384,6 +384,12 @@ end pointer(page) == Ptr{Nothing}(0) && return end + + function _clear_pager!(pager) + for (pid, _) in pager.owned_pages + remove_page(pager, pid) + end + end """ @test lint_test(source, "Line 3, column 10: `fetch` should be used with extreme caution.") @@ -394,6 +400,8 @@ end @test lint_test(source, "Line 17, column 20: `Atomic` should be used with extreme caution.") @test lint_test(source, "Line 19, column 22: `Ptr` should be used with extreme caution.") + + @test lint_test(source, "Line 24, column 9: `remove_page` should be used with extreme caution.") end @testset "Array with no specific type 01" begin From a73dec8a11e57ba7f1e8166a5170bc7eed79bbdb Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Mon, 4 Mar 2024 17:15:14 +0100 Subject: [PATCH 5/8] Channel --- src/linting/extended_checks.jl | 4 ++++ test/rai_rules_tests.jl | 12 +++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index fbd66e5..a195082 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -67,6 +67,7 @@ struct Threads_Extension <: ExtendedRule end struct Generated_Extension <: ExtendedRule end struct Sync_Extension <: ExtendedRule end struct RemovePage_Extension <: ExtendedRule end +struct Channel_Extension <: ExtendedRule end const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule)) @@ -196,3 +197,6 @@ function check(::Sync_Extension, x::EXPR) end check(::RemovePage_Extension, x::EXPR) = generic_check(x, "remove_page(hole_variable,hole_variable)") + +check(::Channel_Extension, x::EXPR) = generic_check(x, "Channel(hole_variable_star)") + diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index 155a85f..24d93b1 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -362,7 +362,7 @@ end "Line 18, column 5: `wait` should be used with extreme caution.") end - @testset "fetch, @inbounds, Atomic, Ptr, remove_page" begin + @testset "fetch, @inbounds, Atomic, Ptr, remove_page, Channel" begin source = """ function f() fut = Future{Any}() @@ -390,6 +390,12 @@ end remove_page(pager, pid) end end + + function foo() + ch1 = Channel() + ch2 = Channel(10) + return (ch1, ch2) + end """ @test lint_test(source, "Line 3, column 10: `fetch` should be used with extreme caution.") @@ -402,6 +408,10 @@ end @test lint_test(source, "Line 19, column 22: `Ptr` should be used with extreme caution.") @test lint_test(source, "Line 24, column 9: `remove_page` should be used with extreme caution.") + + @test lint_test(source, "Line 29, column 11: `Channel` should be used with extreme caution.") + @test lint_test(source, "Line 30, column 11: `Channel` should be used with extreme caution.") + end @testset "Array with no specific type 01" begin From 9c837486f3f96228692f665391aedce714b60889 Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Mon, 4 Mar 2024 17:23:44 +0100 Subject: [PATCH 6/8] Task --- src/linting/extended_checks.jl | 3 ++- test/rai_rules_tests.jl | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index a195082..58c59fc 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -68,6 +68,7 @@ struct Generated_Extension <: ExtendedRule end struct Sync_Extension <: ExtendedRule end struct RemovePage_Extension <: ExtendedRule end struct Channel_Extension <: ExtendedRule end +struct Task_Extension <: ExtendedRule end const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule)) @@ -197,6 +198,6 @@ function check(::Sync_Extension, x::EXPR) end check(::RemovePage_Extension, x::EXPR) = generic_check(x, "remove_page(hole_variable,hole_variable)") - check(::Channel_Extension, x::EXPR) = generic_check(x, "Channel(hole_variable_star)") +check(::Task_Extension, x::EXPR) = generic_check(x, "Task(hole_variable)") diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index 24d93b1..bf1b558 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -394,6 +394,10 @@ end function foo() ch1 = Channel() ch2 = Channel(10) + + a() = sum(i for i in 1:1000); + b = Task(a); + return (ch1, ch2) end """ @@ -412,6 +416,7 @@ end @test lint_test(source, "Line 29, column 11: `Channel` should be used with extreme caution.") @test lint_test(source, "Line 30, column 11: `Channel` should be used with extreme caution.") + @test lint_test(source, "Line 33, column 9: `Task` should be used with extreme caution.") end @testset "Array with no specific type 01" begin From a877ebeb61dec026a00c04b3319a5e79988e7dc4 Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Mon, 4 Mar 2024 21:27:46 +0100 Subject: [PATCH 7/8] ErrorException --- src/linting/extended_checks.jl | 7 +++++++ test/rai_rules_tests.jl | 9 ++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index 58c59fc..eeb019f 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -69,6 +69,7 @@ struct Sync_Extension <: ExtendedRule end struct RemovePage_Extension <: ExtendedRule end struct Channel_Extension <: ExtendedRule end struct Task_Extension <: ExtendedRule end +struct ErrorException_Extension <: ExtendedRule end const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule)) @@ -201,3 +202,9 @@ check(::RemovePage_Extension, x::EXPR) = generic_check(x, "remove_page(hole_vari check(::Channel_Extension, x::EXPR) = generic_check(x, "Channel(hole_variable_star)") check(::Task_Extension, x::EXPR) = generic_check(x, "Task(hole_variable)") +function check(::ErrorException_Extension, x::EXPR) + generic_check( + x, + "ErrorException(hole_variable_star)", + "Use custom exception instead of the generic `ErrorException`") +end \ No newline at end of file diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index bf1b558..7ba6d87 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -362,7 +362,7 @@ end "Line 18, column 5: `wait` should be used with extreme caution.") end - @testset "fetch, @inbounds, Atomic, Ptr, remove_page, Channel" begin + @testset "fetch, @inbounds, Atomic, Ptr, remove_page, Channel, ErrorException" begin source = """ function f() fut = Future{Any}() @@ -398,8 +398,13 @@ end a() = sum(i for i in 1:1000); b = Task(a); + e = ErrorException("failure") return (ch1, ch2) end + + function bar(x) + throw(ErrorException("My error")) + end """ @test lint_test(source, "Line 3, column 10: `fetch` should be used with extreme caution.") @@ -417,6 +422,8 @@ end @test lint_test(source, "Line 30, column 11: `Channel` should be used with extreme caution.") @test lint_test(source, "Line 33, column 9: `Task` should be used with extreme caution.") + + @test lint_test(source, "Line 40, column 11: Use custom exception instead of the generic `ErrorException`") end @testset "Array with no specific type 01" begin From d1bcfac4a89bf4fa05c31adfc94c20390332b4fa Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Mon, 4 Mar 2024 21:30:58 +0100 Subject: [PATCH 8/8] error(...) --- src/linting/extended_checks.jl | 11 ++++++++++- test/rai_rules_tests.jl | 4 ++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index eeb019f..7b29b36 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -70,6 +70,7 @@ struct RemovePage_Extension <: ExtendedRule end struct Channel_Extension <: ExtendedRule end struct Task_Extension <: ExtendedRule end struct ErrorException_Extension <: ExtendedRule end +struct Error_Extension <: ExtendedRule end const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule)) @@ -207,4 +208,12 @@ function check(::ErrorException_Extension, x::EXPR) x, "ErrorException(hole_variable_star)", "Use custom exception instead of the generic `ErrorException`") -end \ No newline at end of file +end + +function check(::Error_Extension, x::EXPR) + generic_check( + x, + "error(hole_variable)", + "Use custom exception instead of the generic `error(...)`") +end + diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index 7ba6d87..883f9fc 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -405,6 +405,8 @@ end function bar(x) throw(ErrorException("My error")) end + + bar() = error("My fault") """ @test lint_test(source, "Line 3, column 10: `fetch` should be used with extreme caution.") @@ -424,6 +426,8 @@ end @test lint_test(source, "Line 33, column 9: `Task` should be used with extreme caution.") @test lint_test(source, "Line 40, column 11: Use custom exception instead of the generic `ErrorException`") + @test lint_test(source, "Line 43, column 9: Use custom exception instead of the generic `error(...)`") + end @testset "Array with no specific type 01" begin