Skip to content

Conversation

@devmotion
Copy link
Member

This PR replaces strip_linenos (the function, not the macro) with Base.remove_linenums!. This leads to cleaner output of the macros, e.g., on master one gets

julia> @macroexpand @scalar_rule identity(x) 1.0
quote                   
    #= /home/david/.julia/packages/ChainRulesCore/JWrYo/src/rule_definition_tools.jl:94 =#
    if !(identity isa ChainRulesCore.Type) && ChainRulesCore.fieldcount(ChainRulesCore.typeof(identity)) > 0
        #= /home/david/.julia/packages/ChainRulesCore/JWrYo/src/rule_definition_tools.jl:95 =#
        ChainRulesCore.throw(ChainRulesCore.ArgumentError("@scalar_rule cannot be used on closures/functors (such as $(identity))"))
    end
    #= /home/david/.julia/packages/ChainRulesCore/JWrYo/src/rule_definition_tools.jl:100 =#
    begin
        nothing
        function (ChainRulesCore.ChainRulesCore).frule((ChainRulesCore._, var"##Δ1#253"), ::ChainRulesCore.typeof(identity), x::Number)
            nothing
            nothing
            #= REPL[4]:1 =#
            nothing
            Ω = identity(x)
            nothing
            nothing
            nothing
            return (Ω, 1.0var"##Δ1#253")
        end
    end
    #= /home/david/.julia/packages/ChainRulesCore/JWrYo/src/rule_definition_tools.jl:101 =#
    begin
        nothing
        function (ChainRulesCore.ChainRulesCore).rrule(::ChainRulesCore.typeof(identity), x::Number)
            nothing
            nothing
            #= REPL[4]:1 =#
            nothing
            Ω = identity(x)
            nothing
            nothing
            nothing
            return (Ω, begin
                        nothing
                        function identity_pullback(var"##Δ1#254")
                            $(Expr(:meta, :inline))
                            nothing
                            nothing
                            #= REPL[4]:1 =#
                            nothing
                            return (ChainRulesCore.NO_FIELDS, ChainRulesCore.conj(1.0) * var"##Δ1#254")
                        end
                    end)
        end
    end
end

whereas this PR yields

julia> @macroexpand @scalar_rule identity(x) 1.0
quote
    #= /home/david/.julia/dev/ChainRulesCore/src/rule_definition_tools.jl:91 =#
    if !(identity isa ChainRulesCore.Type) && ChainRulesCore.fieldcount(ChainRulesCore.typeof(identity)) > 0
        #= /home/david/.julia/dev/ChainRulesCore/src/rule_definition_tools.jl:92 =#
        ChainRulesCore.throw(ChainRulesCore.ArgumentError("@scalar_rule cannot be used on closures/functors (such as $(identity))"))
    end
    #= /home/david/.julia/dev/ChainRulesCore/src/rule_definition_tools.jl:97 =#
    begin
        function (ChainRulesCore.ChainRulesCore).frule((ChainRulesCore._, var"##Δ1#259"), ::ChainRulesCore.typeof(identity), x::Number)
            #= REPL[14]:1 =#
            Ω = identity(x)
            nothing
            return (Ω, 1.0var"##Δ1#259")
        end
    end
    #= /home/david/.julia/dev/ChainRulesCore/src/rule_definition_tools.jl:98 =#
    begin
        function (ChainRulesCore.ChainRulesCore).rrule(::ChainRulesCore.typeof(identity), x::Number)
            #= REPL[14]:1 =#
            Ω = identity(x)
            nothing
            return (Ω, begin
                        function identity_pullback(var"##Δ1#260")
                            $(Expr(:meta, :inline))
                            #= REPL[14]:1 =#
                            return (ChainRulesCore.NO_FIELDS, ChainRulesCore.conj(1.0) * var"##Δ1#260")                        end
                    end)
        end
    end
end

Additionally, Base.remove_linenums! removes not only LineNumberNodes but also Expr(:line, ...): https://github.com/JuliaLang/julia/blob/5d4eaca0c9fa3d555c79dbacdccb9169fdf64b65/base/expr.jl#L349

Copy link
Member

@oxinabox oxinabox left a comment

Choose a reason for hiding this comment

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

Thanks.
I did think there was something in base for this.
Can you bump the version also so we can tag a release?

@devmotion
Copy link
Member Author

Sure, I updated the version number.

@codecov-io
Copy link

codecov-io commented Mar 28, 2021

Codecov Report

Merging #323 (643e140) into master (6c2d1fb) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #323      +/-   ##
==========================================
- Coverage   89.83%   89.76%   -0.07%     
==========================================
  Files          13       13              
  Lines         472      469       -3     
==========================================
- Hits          424      421       -3     
  Misses         48       48              
Impacted Files Coverage Δ
src/rule_definition_tools.jl 96.03% <100.00%> (-0.10%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c2d1fb...643e140. Read the comment docs.

@oxinabox oxinabox merged commit d675be3 into JuliaDiff:master Mar 29, 2021
@devmotion devmotion deleted the dw/remove_linenums branch March 29, 2021 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants