From 851e5db5cd89e8638f9b1af74d3ce5a1a3903850 Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Thu, 29 Feb 2024 12:53:26 +0100 Subject: [PATCH 1/4] fetch, inbounds, Atomic --- src/linting/extended_checks.jl | 14 ++++++++++++++ test/rai_rules_tests.jl | 30 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index 5b34e1e..4799ec6 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -56,6 +56,9 @@ struct Unlock_Extension <: ExtendedRule end struct Yield_Extension <: ExtendedRule end struct Sleep_Extension <: ExtendedRule end struct Mmap_Extension <: ExtendedRule end +struct Fetch_Extension <: ExtendedRule end +struct Inbounds_Extension <: ExtendedRule end +struct Atomic_Extension <: ExtendedRule end const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule)) @@ -139,3 +142,14 @@ function check(::Mmap_Extension, x::EXPR) generic_check(x, "Mmap.mmap(hole_variable_star)", "`mmap` should be used with extreme caution.") end +check(::Fetch_Extension, x::EXPR) = generic_check(x, "fetch(hole_variable)", "`fetch` should be used with extreme caution.") +check(::Inbounds_Extension, x::EXPR) = generic_check(x, "@inbounds hole_variable", "`@inbounds` should be used with extreme caution.") + +function check(::Atomic_Extension, x::EXPR) + msg = "`Atomic` should be used with extreme caution." + generic_check(x, "Atomic(hole_variable_star)", msg) + generic_check(x, "Atomic{hole_variable}(hole_variable_star)", msg) + generic_check(x, "Threads.Atomic(hole_variable_star)", msg) + generic_check(x, "Threads.Atomic{hole_variable}(hole_variable_star)", msg) +end + diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index fa7442d..4fefc9c 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -402,6 +402,36 @@ end @test lint_test(source, "Line 13, column 10: `mmap` should be used with extreme caution.") end + + @testset "fetch, @inbounds, Atomic" begin + source = """ + function f() + fut = Future{Any}() + r1 = fetch(fut) + + @inbounds begin + at_end(iter) && return 0 + i = 1 + set_from_tuple!(columns_tuple, 1, iter_tuple(iter)) + while next!(iter) && i < cap + i += 1 + set_from_tuple!(columns_tuple, i, iter_tuple(iter)) + end + return i + end + num_created1 = Threads.Atomic{Int}(0); + num_created2 = Atomic{Int}(0); + num_created3 = Atomic(0); + end + """ + + @test lint_test(source, "Line 3, column 10: `fetch` should be used with extreme caution.") + @test lint_test(source, "Line 5, column 5: `@inbounds` should be used with extreme caution.") + + @test lint_test(source, "Line 15, column 20: `Atomic` should be used with extreme caution.") + @test lint_test(source, "Line 16, column 20: `Atomic` should be used with extreme caution.") + @test lint_test(source, "Line 17, column 20: `Atomic` should be used with extreme caution.") + end end @testset "Comparison" begin From c49d33688d1279db2ddffa9ade5092f7ef2692af Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Thu, 29 Feb 2024 12:59:33 +0100 Subject: [PATCH 2/4] fixing syntax error --- test/rai_rules_tests.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index 64e2aec..ef59b6a 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -413,7 +413,6 @@ end "Line 16, column 12: `Future` should be used with extreme caution.") @test lint_test(source, "Line 18, column 5: `wait` should be used with extreme caution.") - end end @testset "fetch, @inbounds, Atomic" begin @@ -447,6 +446,7 @@ end end end + @testset "Comparison" begin t(s1, s2) = comp(CSTParser.parse(s1), CSTParser.parse(s2)) @test t("Threads.nthreads()", "Threads.nthreads()") From 39210dc717bca2c580718d7bfa7bc2f243611c98 Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Thu, 29 Feb 2024 14:19:46 +0100 Subject: [PATCH 3/4] Simplifying the generation of error msg. --- src/linting/extended_checks.jl | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index cca1359..9ded3ad 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -61,6 +61,8 @@ struct Wait_Extension <: ExtendedRule end struct Fetch_Extension <: ExtendedRule end struct Inbounds_Extension <: ExtendedRule end struct Atomic_Extension <: ExtendedRule end +struct Ptr_Extension <: ExtendedRule end + const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule)) @@ -83,11 +85,17 @@ function get_oracle_ast(template_code::String) end does_match(x::EXPR, template_code::String) = comp(x, get_oracle_ast(template_code)) -function generic_check(x::EXPR, template_code::String, error_code) - error_code isa String && get!(error_msgs, template_code, error_code) - does_match(x, template_code) && seterror!(x, error_code) +function generic_check(x::EXPR, template_code::String, error_msg) + error_msg isa String && get!(error_msgs, template_code, error_msg) + does_match(x, template_code) && seterror!(x, error_msg) +end + +function generic_check(x::EXPR, template_code::String) + keyword = first(split(template_code, ['(', '{', ' '])) + return generic_check(x, template_code, "`$(keyword)` should be used with extreme caution.") end + # Useful for rules that do not need markers check(t::Any, x::EXPR, markers::Dict{Symbol,Symbol}) = check(t, x) @@ -136,16 +144,16 @@ function check(::Lock_Extension, x::EXPR) generic_check(x, "Base.@lock hole_variable hole_variable", msg) end -check(::Unlock_Extension, x::EXPR) = generic_check(x, "unlock(hole_variable)", "`unlock` should be used with extreme caution.") -check(::Yield_Extension, x::EXPR) = generic_check(x, "yield()", "`yield` should be used with extreme caution.") -check(::Sleep_Extension, x::EXPR) = generic_check(x, "sleep(hole_variable)", "`sleep` should be used with extreme caution.") +check(::Unlock_Extension, x::EXPR) = generic_check(x, "unlock(hole_variable)") +check(::Yield_Extension, x::EXPR) = generic_check(x, "yield()") +check(::Sleep_Extension, x::EXPR) = generic_check(x, "sleep(hole_variable)") function check(::Mmap_Extension, x::EXPR) - generic_check(x, "mmap(hole_variable_star)", "`mmap` should be used with extreme caution.") + generic_check(x, "mmap(hole_variable_star)") generic_check(x, "Mmap.mmap(hole_variable_star)", "`mmap` should be used with extreme caution.") end -check(::Fetch_Extension, x::EXPR) = generic_check(x, "fetch(hole_variable)", "`fetch` should be used with extreme caution.") -check(::Inbounds_Extension, x::EXPR) = generic_check(x, "@inbounds hole_variable", "`@inbounds` should be used with extreme caution.") +check(::Fetch_Extension, x::EXPR) = generic_check(x, "fetch(hole_variable)") +check(::Inbounds_Extension, x::EXPR) = generic_check(x, "@inbounds hole_variable") function check(::Atomic_Extension, x::EXPR) msg = "`Atomic` should be used with extreme caution." @@ -156,8 +164,10 @@ function check(::Atomic_Extension, x::EXPR) end function check(::Future_Extension, x::EXPR) - generic_check(x, "Future{hole_variable}(hole_variable_star)", "`Future` should be used with extreme caution.") - generic_check(x, "Future(hole_variable_star)", "`Future` should be used with extreme caution.") + generic_check(x, "Future{hole_variable}(hole_variable_star)") + generic_check(x, "Future(hole_variable_star)") end -check(::Wait_Extension, x::EXPR) = generic_check(x, "wait(hole_variable)", "`wait` should be used with extreme caution.") +check(::Wait_Extension, x::EXPR) = generic_check(x, "wait(hole_variable)") + +check(::Ptr_Extension, x::EXPR) = generic_check(x, "Ptr{hole_variable}(hole_variable)") \ No newline at end of file From e7cfa78bc2829200a6d419e86453187df724c89b Mon Sep 17 00:00:00 2001 From: Alexandre Bergel Date: Thu, 29 Feb 2024 14:23:45 +0100 Subject: [PATCH 4/4] Added Ptr --- src/linting/extended_checks.jl | 1 - test/rai_rules_tests.jl | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/linting/extended_checks.jl b/src/linting/extended_checks.jl index 9ded3ad..bbea557 100644 --- a/src/linting/extended_checks.jl +++ b/src/linting/extended_checks.jl @@ -169,5 +169,4 @@ function check(::Future_Extension, x::EXPR) end check(::Wait_Extension, x::EXPR) = generic_check(x, "wait(hole_variable)") - check(::Ptr_Extension, x::EXPR) = generic_check(x, "Ptr{hole_variable}(hole_variable)") \ No newline at end of file diff --git a/test/rai_rules_tests.jl b/test/rai_rules_tests.jl index ef59b6a..585cfbe 100644 --- a/test/rai_rules_tests.jl +++ b/test/rai_rules_tests.jl @@ -415,7 +415,7 @@ end "Line 18, column 5: `wait` should be used with extreme caution.") end - @testset "fetch, @inbounds, Atomic" begin + @testset "fetch, @inbounds, Atomic, Ptr" begin source = """ function f() fut = Future{Any}() @@ -434,6 +434,8 @@ end num_created1 = Threads.Atomic{Int}(0); num_created2 = Atomic{Int}(0); num_created3 = Atomic(0); + + pointer(page) == Ptr{Nothing}(0) && return end """ @@ -443,6 +445,8 @@ end @test lint_test(source, "Line 15, column 20: `Atomic` should be used with extreme caution.") @test lint_test(source, "Line 16, column 20: `Atomic` should be used with extreme caution.") @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.") end end