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

Duplicated error messages during doctests #42

Open
jakobjpeters opened this issue Jun 24, 2024 · 9 comments
Open

Duplicated error messages during doctests #42

jakobjpeters opened this issue Jun 24, 2024 · 9 comments

Comments

@jakobjpeters
Copy link
Contributor

In a certain case, the error message from failed doctests of methods wrapped in @stable are duplicated. I've seen this before, but it was really challenging to debug and I couldn't pin down the source. Here is an example repository and CI job that demonstrate it. In the CI job, note that the error for f and g is displayed once but the error for h is displayed twice.

Works as expected

"""
```jldoctest
julia> f()
```
"""
@stable f() = 1

"""
```jldoctest
julia> g()
```
"""
@stable begin g() = 2 end

Duplicates error messages

@stable begin
"""
```jldoctest
julia> h()
```
"""
h() = 3
end
```
@jakobjpeters
Copy link
Contributor Author

jakobjpeters commented Jun 24, 2024

It looks like the docstring gets attached to the simulator, and then for some reason Documenter.jl also tests that docstring even though it isn't included in the manual. Then, the function name isn't displayed in the error and the line numbers match, so it's difficult to tell what's going on.

julia> @macroexpand @stable begin
       """
       ```jldoctest
       julia> f()
       ```
       """
       f() = 1
       end
quote
    #= REPL[6]:2 =#
    begin
        #= /home/jakob/.julia/packages/DispatchDoctor/bx82n/src/stabilization.jl:316 =#
        begin
            function var"##f_simulator#231"(; )
                #= REPL[6]:7 =#
                1
            end
            (Base.Docs.doc!)(Main, (Base.Docs.Binding)(Main, Symbol("##f_simulator#231")), (Base.Docs.docstr)((Core.svec)("```jldoctest\njulia> f()\n```\n"), (Dict{Symbol, Any})(:path => "REPL[6]", :linenumber => 2, :module => Main)), Union{Tuple{}})
        end
        #= /home/jakob/.julia/packages/DispatchDoctor/bx82n/src/stabilization.jl:317 =#
        begin
            $(Expr(:meta, :doc))
            begin
                function f(; )
                    #= /home/jakob/.julia/packages/DispatchDoctor/bx82n/src/stabilization.jl:298 =#
                    var"##f_return_type#232" = (Base).promote_op(var"##f_simulator#231", map(DispatchDoctor._Utils.specializing_typeof, ())...)
                    #= /home/jakob/.julia/packages/DispatchDoctor/bx82n/src/stabilization.jl:299 =#
                    if (DispatchDoctor._Utils.type_instability)(var"##f_return_type#232") && (!((DispatchDoctor._Interactions.ignore_function)(f)) && (DispatchDoctor._RuntimeChecks.checking_enabled)())
                        #= /home/jakob/.julia/packages/DispatchDoctor/bx82n/src/stabilization.jl:300 =#
                        throw((TypeInstabilityError)("`f`", "REPL[6]:7", (), (;), (DispatchDoctor._Stabilization._construct_pairs)((), ()), var"##f_return_type#232"))
                    end
                    #= /home/jakob/.julia/packages/DispatchDoctor/bx82n/src/stabilization.jl:303 =#
                    begin
                        #= REPL[6]:7 =#
                        1
                    end
                end
                (Base.Docs.doc!)(Main, (Base.Docs.Binding)(Main, :f), (Base.Docs.docstr)((Core.svec)("```jldoctest\njulia> f()\n```\n"), (Dict{Symbol, Any})(:path => "REPL[6]", :linenumber => 2, :module => Main)), Union{Tuple{}})
            end
        end
    end
end

@MilesCranmer
Copy link
Owner

Thanks for the report. Does this also occur if you set default_codegen_level="min"?

I wonder if we should try to strip the line numbers from the simulator function. As you can see, both f and f_simulator have the line #= REPL[6]:7 =# (I think this is also what confuses code coverage for default_codegen_level="debug"). Maybe Documenter is somehow attaching it based on that?

This is the generated call:

final_ex = quote
$(func_simulator_ex)
$(Base).@__doc__ $(func_ex)
end

I wonder if the simulator function needs to be defined second for this to work? (And the regular function's symbol repeated at the end so it is the return value)

@jakobjpeters
Copy link
Contributor Author

jakobjpeters commented Jun 24, 2024

Both default_codegen_level = "min" and removing the line number nodes still exhibit this issue.

EDIT: And also independently of each other too.

eval(Base.remove_linenums!(@macroexpand @stable default_codegen_level = "min" begin
"""
```jldoctest
julia> i()
```
"""
i() = 4
end))

@MilesCranmer
Copy link
Owner

Hm... Very weird. I wonder if MacroTools.jl is adding that (Base.Docs.doc!) here?

@jakobjpeters
Copy link
Contributor Author

jakobjpeters commented Jun 24, 2024

Worth looking into! Removing the simulator's (Base.Docs.doc!) fixes the problem.

const x = @macroexpand @stable begin
"""
```jldoctest
julia> j()
```
"""
j() = 5
end

pop!(x.args[2].args[2].args)
eval(x)

@jakobjpeters
Copy link
Contributor Author

Assuming DontPropagateMacro means that the macro isn't added to macros_to_use, then @doc is incorrectly branching into CompatibleMacro instead. The macro is being passed to get_macro_behavior as a GlobalRef, which defaults to CompatibleMacro. I don't think there's a public API to get the symbol out though?

As a test, switching the fallback behavior to DontPropagateMacro indeed fixes the issue.

@MilesCranmer
Copy link
Owner

Does putting @doc as DontPropagateMacro fix it? Or does it need to be default for all macros?

@jakobjpeters
Copy link
Contributor Author

jakobjpeters commented Jun 26, 2024

@doc is already DontPropagateMacro.

Symbol("@doc") => DontPropagateMacro, # Core

The problem is that get_macro_behavior is passed a GlobalRef (containing @doc), which dispatches to this fallback.

get_macro_behavior(_) = CompatibleMacro

@MilesCranmer
Copy link
Owner

I see, thanks. One thing I’m confused about is I though Base.@__doc__ would already take care of this. So it’s weird this is happening. Maybe the _stabilize_all needs a branch for GlobalRef?

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

No branches or pull requests

2 participants