Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

code quality improvements from JET analysis #159

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "MacroTools"
uuid = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
version = "0.5.6"
version = "0.5.7"

[deps]
Markdown = "d6f4376e-aef5-505a-96c1-9c027394607a"
Expand Down
6 changes: 5 additions & 1 deletion src/match/match.jl
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ isbinding(s) = false
isbinding(s::Symbol) = occursin(r"[^_]_(_str)?$", string(s))

function bname(s::Symbol)
Symbol(Base.match(r"^@?(.*?)_+(_str)?$", string(s)).captures[1])
return @static if isdefined(Base, :AbstractMatch)
Symbol((Base.match(r"^@?(.*?)_+(_str)?$", string(s))::AbstractMatch).captures[1])
else
Symbol((Base.match(r"^@?(.*?)_+(_str)?$", string(s))::Base.RegexMatch).captures[1])
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change just replaces one runtime error by another, right?

julia> nothing.captures
ERROR: type Nothing has no field captures

julia> (nothing::AbstractMatch).captures
ERROR: TypeError: in typeassert, expected AbstractMatch, got a value of type Nothing

What's the point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIU that runtime error won't happen (the regex always matches bname's argument), thus this annotation.

end

function match_inner(pat, ex, env)
Expand Down
6 changes: 3 additions & 3 deletions src/structdef.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,23 @@ function splitstructdef(ex)
elseif @capture(ex, mutable struct header_ body__ end)
d[:mutable] = true
else
parse_error(ex)
throw(ArgumentError("can't split $(repr(ex))"))
end

if @capture header nameparam_ <: super_
nothing
elseif @capture header nameparam_
super = :Any
else
parse_error(ex)
throw(ArgumentError("can't split $(repr(ex))"))
end
d[:supertype] = super
if @capture nameparam name_{param__}
nothing
elseif @capture nameparam name_
param = []
else
parse_error(ex)
throw(ArgumentError("can't split $(repr(ex))"))
end
d[:name] = name
d[:params] = param
Expand Down
13 changes: 3 additions & 10 deletions src/utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -417,13 +417,6 @@ function combinearg(arg_name, arg_type, is_splat, default)
return default === nothing ? a2 : Expr(:kw, a2, default)
end


macro splitcombine(fundef)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macro with the same name is defined in test file, and this
could be removed.
This definition is also timed, especially rebuilddef is no longer defined.

dict = splitdef(fundef)
esc(rebuilddef(striplines(dict)))
end


"""
splitarg(arg)

Expand All @@ -444,11 +437,11 @@ See also: [`combinearg`](@ref)
"""
function splitarg(arg_expr)
splitvar(arg) =
@match arg begin
(@match arg begin
::T_ => (nothing, T)
name_::T_ => (name, T)
x_ => (x, :Any)
end
x_ => (x, :Any) # capture every pattern
end)::NTuple{2,Any}
(is_splat = @capture(arg_expr, arg_expr2_...)) || (arg_expr2 = arg_expr)
if @capture(arg_expr2, arg_ = default_)
@assert default !== nothing "splitarg cannot handle `nothing` as a default. Use a quoted `nothing` if possible. (MacroTools#35)"
Expand Down
1 change: 1 addition & 0 deletions test/split.jl
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,5 @@ end
@test first(constructors) ==
:((S(a::A) where A) = new{A}()) |> MacroTools.flatten

@test_throws ArgumentError splitstructdef(:(call_ex(arg)))
end