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.

Sign up for GitHub

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

Jump to bottom

over-eager method deletion? #137

Closed
vtjnash opened this issue Aug 9, 2018 · 4 comments · Fixed by #180
Closed

over-eager method deletion? #137

vtjnash opened this issue Aug 9, 2018 · 4 comments · Fixed by #180

Comments

@vtjnash
Copy link
Collaborator

vtjnash commented Aug 9, 2018

I had some code like this in a submodule, and it seems like Revise.jl deleted these method when I loaded this package (and touched the file). Sorry I can't try to check it further as an MRE right now.

let
    indent = "  ︰  "^10
    indent_z = collect(eachindex(indent))
    global function print_indent(io::IO, depth::Int)
        depth < 1 && return
        while depth > length(indent_z)
            print(io, indent)
            depth -= length(indent_z)
        end
        print(io, SubString(indent, 1, indent_z[depth]))
    end
end

function process(data::Vector{<:Unsigned}=UInt[], lidict::Set=Base.Set(data))
end
@timholy
Copy link
Owner

timholy commented Aug 9, 2018

Thanks for the example. Sadly doesn't trigger for me:

julia> includet("nash.jl")

julia> process
process (generic function with 3 methods)

julia> methods(process)
# 3 methods for generic function "process":
[1] process() in Main at /tmp/nash.jl:15
[2] process(data::Array{#s24,1} where #s24<:Unsigned) in Main at /tmp/nash.jl:15
[3] process(data::Array{#s25,1} where #s25<:Unsigned, lidict::Set) in Main at /tmp/nash.jl:15

julia> print_indent
print_indent (generic function with 1 method)

shell> touch nash.jl

julia> 

julia> print_indent
print_indent (generic function with 1 method)

julia> methods(process)
# 3 methods for generic function "process":
[1] process() in Main at /tmp/nash.jl:15
[2] process(data::Array{#s24,1} where #s24<:Unsigned) in Main at /tmp/nash.jl:15
[3] process(data::Array{#s25,1} where #s25<:Unsigned, lidict::Set) in Main at /tmp/nash.jl:15

julia> print_indent(stdout, 5)
  ︰

True also if I place the above inside

module Mod1

module Mod2
<body>

end

end

Happy to go over these in person together when you have time.

@vtjnash
Copy link
Collaborator Author

vtjnash commented Aug 10, 2018 •
edited
Loading

Could it be related that these were precompiled into stdlib?

@timholy
Copy link
Owner

timholy commented Aug 19, 2018

Maybe. #163 might help debug stuff.

timholy added a commit that referenced this issue Sep 5, 2018
@timholy
Swap the order of deletion and evaluation
3bf2270 
This works around JuliaLang/julia#28899
and probably fixes #137

New tests:

- test JuliaLang/julia#28899
- test for signature-changes with a gensymmed type
- test that changes that do not fundamentally change the type signature
  do not result in inadvertent deletion (probably #137).
timholy added a commit that referenced this issue Sep 5, 2018
@timholy
Swap the order of deletion and evaluation
e2237b0 
This works around JuliaLang/julia#28899
and probably fixes #137

New tests:

- test JuliaLang/julia#28899
- test for signature-changes with a gensymmed type
- test that changes that do not fundamentally change the type signature
  do not result in inadvertent deletion (probably #137).
@timholy timholy mentioned this issue Sep 5, 2018
Merged
@timholy
Copy link
Owner

timholy commented Sep 5, 2018

I'm calling this fixed by #180. If you encounter future troubles you can always have logging on by default and consequently not worry so much about whether you can find a MWE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Swap the order of deletion and evaluation timholy/Revise.jl
2 participants
@vtjnash @timholy