Skip to content

Commit

Permalink
Merge pull request #23 from RelationalAI/adding-threads
Browse files Browse the repository at this point in the history 
Adding threads
  • Loading branch information
@bergel
bergel authored Mar 4, 2024
2 parents 9ffd483 + d1bcfac commit a1428a4
Show file tree
Hide file tree
Showing 2 changed files with 195 additions and 90 deletions.
43 changes: 41 additions & 2 deletions src/linting/extended_checks.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,14 @@ 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
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
struct ErrorException_Extension <: ExtendedRule end
struct Error_Extension <: ExtendedRule end

const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule))

Expand Down Expand Up @@ -178,3 +184,36 @@ 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

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

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)")

function check(::ErrorException_Extension, x::EXPR)
generic_check(
x,
"ErrorException(hole_variable_star)",
"Use custom exception instead of the generic `ErrorException`")
end

function check(::Error_Extension, x::EXPR)
generic_check(
x,
"error(hole_variable)",
"Use custom exception instead of the generic `error(...)`")
end

242 changes: 154 additions & 88 deletions test/rai_rules_tests.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,104 +64,51 @@ end
"Line 2, column 5: Macro `@cfunction` should not be used.")
end

@testset "@lock" begin
@testset "@lock, @threads, @generated" 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
return nothing
end
"""
@test lint_has_error_test(source)
@test lint_test(source,
"Line 2, column 5: `@lock` 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
""")
Threads.@threads for (e, v) in slots
@test e[] == v
free_slot!(pool, Blob{Nothing}(e))
end
return nothing
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
@generated function _empty_vector(::Type{T}) where {T}
vec = T[]
return vec
end
@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)
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,
"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.")
@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

Expand Down Expand Up @@ -415,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, Channel, ErrorException" begin
source = """
function f()
fut = Future{Any}()
Expand All @@ -437,6 +384,29 @@ end
pointer(page) == Ptr{Nothing}(0) && return
end
function _clear_pager!(pager)
for (pid, _) in pager.owned_pages
remove_page(pager, pid)
end
end
function foo()
ch1 = Channel()
ch2 = Channel(10)
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
bar() = error("My fault")
"""

@test lint_test(source, "Line 3, column 10: `fetch` should be used with extreme caution.")
Expand All @@ -447,6 +417,17 @@ 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.")

@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.")

@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
Expand Down Expand Up @@ -875,4 +856,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

0 comments on commit a1428a4

Please sign in to comment.