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

Use depth-limited type printing #568

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

charleskawczynski
Copy link

@charleskawczynski charleskawczynski commented Apr 25, 2024

Closes #566.

@charleskawczynski charleskawczynski force-pushed the ck/depth_limited_type_printing branch from 0aacc2c to 5b42185 Compare April 25, 2024 15:53
@Zentrik
Copy link
Collaborator

Zentrik commented Apr 25, 2024

TypedSyntax is a separate package that can work without Cthulhu, so the print.jl file needs to be in TypedSyntax and can't rely on a global in Cthulhu.
It looks like print.jl is only a line or 2 so seems simplest to just put the code where T_str = string(T) is currently, otherwise you should probably add @nospecialize to the arguments of the new functions given the function T_str = string(T) is contained in does.

@charleskawczynski
Copy link
Author

TypedSyntax is a separate package that can work without Cthulhu, so the print.jl file needs to be in TypedSyntax and can't rely on a global in Cthulhu. It looks like print.jl is only a line or 2 so seems simplest to just put the code where T_str = string(T) is currently, otherwise you should probably add @nospecialize to the arguments of the new functions given the function T_str = string(T) is contained in does.

I still never managed to exercise show_annotation, so it's difficult for me to debug this, but moving it there seems like the right way forward, and we can then figure out how to pass CONFIG.type_depth_limit to it through Base.printstyled

@charleskawczynski charleskawczynski force-pushed the ck/depth_limited_type_printing branch 2 times, most recently from d23cfff to 6899a17 Compare April 25, 2024 16:43
@charleskawczynski charleskawczynski marked this pull request as ready for review April 25, 2024 16:52
@charleskawczynski
Copy link
Author

I think that this is now working. From the test added in the test suite:

Screen Shot 2024-04-25 at 12 53 27 PM

@charleskawczynski
Copy link
Author

On main, it looks like this:

Screen Shot 2024-04-25 at 12 55 39 PM

Add helper

Use depth-limited type printing
@charleskawczynski charleskawczynski force-pushed the ck/depth_limited_type_printing branch from 6899a17 to e542d59 Compare April 25, 2024 16:57
@charleskawczynski
Copy link
Author

I'm not sure what the best way is forward for testing, @vchuravy?

TypedSyntax/src/show.jl Outdated Show resolved Hide resolved
src/codeview.jl Outdated Show resolved Hide resolved
@Zentrik
Copy link
Collaborator

Zentrik commented Apr 25, 2024

Thanks for this, I've left a couple comments as these seems unnecessarily complicated right now.

@Zentrik
Copy link
Collaborator

Zentrik commented Apr 26, 2024

For the tests, https://github.com/JuliaDebug/Cthulhu.jl/blob/master/test/test_codeview.jl looks like a good place to put it and hopefully the other tests in that file will help you in writing one for this.
E.g. I think you should be able to adapt

let # optimize code
(; src, infos, mi, rt, exct, effects, slottypes) = @eval Module() begin
const globalvar = Ref(42)
$cthulhu_info() do
a = sin(globalvar[])
b = sin(undefvar)
return (a, b)
end
end
function prints(; kwargs...)
io = IOBuffer()
Cthulhu.cthulhu_typed(io, :none, src, rt, exct, effects, mi; kwargs...)
return String(take!(io))
end
let # by default, should print every statement
s = prints()
@test occursin("globalvar", s)
@test occursin("undefvar", s)
end
let # should omit type stable statements
s = prints(; hide_type_stable=true)
@test !occursin("globalvar", s)
@test occursin("undefvar", s)
end
end
to test this.

You might want to disable this feature by default for tests, maybe in https://github.com/JuliaDebug/Cthulhu.jl/blob/master/test/runtests.jl to prevent this from breaking existing tests.

@charleskawczynski
Copy link
Author

For the tests, https://github.com/JuliaDebug/Cthulhu.jl/blob/master/test/test_codeview.jl looks like a good place to put it and hopefully the other tests in that file will help you in writing one for this. E.g. I think you should be able to adapt...

Hm, my first attempt doesn't seem to be limiting the types:

#=
using Revise; include(joinpath("test", "test_depth_limited_type_printing.jl"))
=#
import Cthulhu

Base.@kwdef struct Nested{A,B}
    num::Int = 1
end
bar(x) = rand() > 0.5 ? x : Any[0][1]
mysum(x) = sum(y-> bar(x.num), 1:5; init=0)
nest_val(na, nb, ::Val{1}) = Nested{na, nb}()
nest_val(na, nb, ::Val{n}) where {n} = nest_val(Nested{na, nb}, Nested{na, nb}, Val(n-1))
nest_val(na, nb, n::Int) = nest_val(na, nb, Val(n))
nest_val(n) = nest_val(1, 1, n)
const NV = nest_val(5)

# f = nest_val(5)
# a = Any[f];
# mysum(a[1]) # make sure it runs
# Cthulhu.@descend mysum(a[1]) # navigate to sum -> sum, and Nested will be there
using Test
include("setup.jl")
@testset "hide type-stable statements" begin
    let # optimize code
        # f = nest_val(5)
        # a = Any[f];
        # mysum(a[1]) # make sure it runs
        # Cthulhu.@descend mysum(a[1]) # navigate to sum -> sum, and Nested will be there
        (; src, infos, mi, rt, exct, effects, slottypes) = @eval Module() begin
            $cthulhu_info($mysum, ($(typeof(NV)),))
        end;
        function prints(; kwargs...)
            io = IOBuffer()
            ioc = IOContext(io, :maxtypedepth => Cthulhu.CONFIG.type_depth_limit)
            Cthulhu.cthulhu_typed(ioc, :none, src, rt, exct, effects, mi; kwargs...)
            return String(take!(io))
        end;

        let # by default, should print every statement
            s = prints()
            println(s)
            # @test occursin("::Nested{Nested{…}, Nested{…}}", s)
        end
    end
end

Any pointers?

@Zentrik
Copy link
Collaborator

Zentrik commented Apr 30, 2024

I don't see anything obvious but I'm very busy right now so I'm not going to be able to get back to this pr for a few weeks.
Thanks for your work so far, especially in addressing my comments.

@simeonschaub simeonschaub added the enhancement New feature or request label Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use depth-limited type printing, from Julia 1.10
3 participants