Skip to content

Use depth-limited type printing #568

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

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

@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?

@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
@charleskawczynski charleskawczynski force-pushed the ck/depth_limited_type_printing branch from 06db788 to 426f95e Compare March 20, 2025 15:55
@charleskawczynski
Copy link
Author

#566 makes this package virtually unusable for us in some applications. Can someone please take a look at this? This PR does fix the issue, I think the test just needs someone's attention who is familiar with the style of the existing tests (and perhaps for someone to try with VScode integration).

@charleskawczynski
Copy link
Author

Actually, it looks like some of the types are printed using InteractiveTools.warntype_type_printer, so we need to fix this in julialang. At which point, we might as well and try to fix JuliaLang/julia#55952 first, and spread its use to InteractiveUtils, JET and here. I think that's probably a better course of action..

Copy link
Collaborator

@Zentrik Zentrik left a comment

Choose a reason for hiding this comment

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

Sorry, for dropping the ball on this. I had a brief look and this lgtm

@charleskawczynski
Copy link
Author

charleskawczynski commented Mar 26, 2025

Sorry, for dropping the ball on this. I had a brief look and this lgtm

It’s alright. Honestly, I think we need to fix JuliaLang/julia#55952 and formulate the functions to return a depth-limited string (for re-useability ), since one (or more) of the types is printed with InteractiveUtils

@serenity4
Copy link
Collaborator

serenity4 commented Mar 27, 2025

I'd prefer if we had a UI option to increase/decrease the type depth limit (or directly set a number). Most of the time, it is preferable to get all of the information (even if harder to read) rather than incomplete information with no way to retrieve it; the exception being of course the case where it completely makes it unusable due to heavily nested types spamming the terminal. I am notably thinking of cases where perhaps one or two expressions may have heavily nested types in a function, and these being of interest (e.g. to detect non-concrete types); that'd be frustrating to only see part of it with really no way to dig deeper while that is the main point of Cthulhu:

julia> function f(x)
         x[][] + 1
       end
f (generic function with 1 method)

julia> @descend f(Ref(Ref(5)))
f(x) @ Main ~/.julia/dev/Cthulhu/script.jl:4
4 function f(x::Base.RefValue{Base.RefValue{…}})::Int64
5   (x::Base.RefValue{Base.RefValue{…}}[]::Base.RefValue{Int64}[]::Int64 + 1)::Int64
6 end
Select a call to descend into or  to ascend. [q]uit. [b]ookmark.
Toggles: [w]arn, [h]ide type-stable statements, [t]ype annotations, [s]yntax highlight for Source/LLVM/Native, [j]ump to source always, [v]scode: inlay types, [V]scode: diagnostics.
Show: [S]ource code, [A]ST, [T]yped code, [L]LVM IR, [N]ative code
Actions: [E]dit source code, [R]evise and redisplay
 • x::Base.RefValue{Base.RefValue{…}}[]
   x::Base.RefValue{Base.RefValue{…}}[]::Base.RefValue{Int64}[]
   x::Base.RefValue{Base.RefValue{…}}[]::Base.RefValue{Int64}[]::Int64 + 1
   

Then if we have a UI option, we could set the default type depth limit to something higher, perhaps 6 or 7, to keep roughly the same behavior as currently for most cases, while avoiding non-negligible performance/readability issues for very heavily nested types. What do you think?

@charleskawczynski
Copy link
Author

I'd prefer if we had a UI option to increase/decrease the type depth limit (or directly set a number). Most of the time, it is preferable to get all of the information (even if harder to read) rather than incomplete information with no way to retrieve it; the exception being of course the case where it completely makes it unusable due to heavily nested types spamming the terminal. I am notably thinking of cases where perhaps one or two expressions may have heavily nested types in a function, and these being of interest (e.g. to detect non-concrete types); that'd be frustrating to only see part of it with really no way to dig deeper while that is the main point of Cthulhu:

julia> function f(x)
         x[][] + 1
       end
f (generic function with 1 method)

julia> @descend f(Ref(Ref(5)))
f(x) @ Main ~/.julia/dev/Cthulhu/script.jl:4
4 function f(x::Base.RefValue{Base.RefValue{…}})::Int64
5   (x::Base.RefValue{Base.RefValue{…}}[]::Base.RefValue{Int64}[]::Int64 + 1)::Int64
6 end
Select a call to descend into or  to ascend. [q]uit. [b]ookmark.
Toggles: [w]arn, [h]ide type-stable statements, [t]ype annotations, [s]yntax highlight for Source/LLVM/Native, [j]ump to source always, [v]scode: inlay types, [V]scode: diagnostics.
Show: [S]ource code, [A]ST, [T]yped code, [L]LVM IR, [N]ative code
Actions: [E]dit source code, [R]evise and redisplay
 • x::Base.RefValue{Base.RefValue{…}}[]
   x::Base.RefValue{Base.RefValue{…}}[]::Base.RefValue{Int64}[]
   x::Base.RefValue{Base.RefValue{…}}[]::Base.RefValue{Int64}[]::Int64 + 1
   

Then if we have a UI option, we could set the default type depth limit to something higher, perhaps 6 or 7, to keep roughly the same behavior as currently for most cases, while avoiding non-negligible performance/readability issues for very heavily nested types. What do you think?

Yeah, that would be nice. I'm not really familiar with how to UI works, so I might need pointers for that. This is why I just configured things so that there is a global module setting that users can change.

Unfortunately, as I mentioned, some of the types are printed in InteractiveTools.warntype_type_printer-- and this is printed with syntax highlighting, so we can't easily/elegantly print this to a buffer and apply depth-limited type printing from within Cthulhu. i.e., we need a fix in julialang.

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
4 participants