Depth-limited printing of types in stacktraces take 2! #49795
Conversation
JeffBezanson
added
needs tests
|
Thanks! I haven't looked in detail, but the principle of doing more things with raw strings is a good one. I'll be happy to give it a thorough review after one issue worthy of discussion: one not-so-obvious goal of my previous implementation was to create an object that could, if desired, be passed to FoldingTrees for interactive exploration of the type. But I am unsure that's a sufficiently important goal to be worth a far more complicated implementation. What do you think? EDIT: Thinking about it more, one might be able to use this implementation and still construct a tree object. Creating a custom IO might have been a case of premature optimization. |
base/show.jl
Outdated
| if get(out, :limit, false)::Bool | ||
| io = IOBuffer() | ||
| if isa(out, IOContext) | ||
| io = IOContext(io, out) | ||
| end | ||
| else | ||
| io = out | ||
| end |
There was a problem hiding this comment.
Type-unstable code like this should generally be avoided. There is no reason to not call the IOContext wrapper unconditionally, and it is incorrect anyways not to wrap it when making an IOBuffer that is supposed to act like the original IO (several properties still get copied over, even if it is not previously an IOContext).
base/show.jl
Outdated
| end | ||
| if c == '{' | ||
| depth += 1 | ||
| elseif in_escape && c == 'm' |
There was a problem hiding this comment.
Why is it safe to assume ANSI parsing and that m is the only valid ANSI code for generic IO?
There was a problem hiding this comment.
Oh excellent, I didn't realize we had ANSIIterator for this!
base/show.jl
Outdated
| elseif in_escape && c == 'm' | ||
| in_escape = false | ||
| end | ||
| end |
There was a problem hiding this comment.
This would fail on a type such as:
julia> Val{'{'}
Val{'{'}
There was a problem hiding this comment.
Overall I really like this. I think we could just solve the stacktrace printing problem and decide later if we want to be able to introspect on types interactively.
It seems like one or perhaps two changes might be necessary:
- detect when braces are not syntactically meaningful (@vtjnash's
Val{'{'}and alsoVal{Symbol("2^{x-y}")}), which I think we could handle withis_quotedsimilar toin_escape - possibly more nuanced handling of escape codes. Since this is only used for printing types---and we tell people not to specialize this---I think the world of possible escapes is limited. If we merge AnnotatedStrings, and a string styling stdlib #49586 that would provide one way of handling it; if you need something that does slightly more validation, you could steal anything you want from https://github.com/JuliaIO/WidthLimitedIO.jl/blob/main/src/ansi_escapes.jl
I'd approve porting over the tests I included and merging this once other concerns are addressed. I'm not sure my PR included the full suite of tests I had in mind, but what I was struggling with (reproducing the character-for-character coloring when the width of the display was not an issue) may not be applicable with the approach taken here.
JeffBezanson
removed
needs tests
JeffBezanson
force-pushed
the
jb/depthlimitty
branch
from
3cab53d to
71ffa02
Compare
|
OK I think this is ready to go. I used the lovely |
JeffBezanson
force-pushed
the
jb/depthlimitty
branch
from
71ffa02 to
47e69fa
Compare
| end | ||
| str = sprint(Base.show_backtrace, st, context = (:limit=>true, :color=>true, :displaysize=>(50,132))) | ||
| @test contains(str, "[2] \e[0m\e[1m(::$F49231{Vector, Val{…}, Vector{…}, NTuple{…}, Int64, Int64, Int64})\e[22m\e[0m\e[1m(\e[22m\e[90ma\e[39m::\e[0mInt64, \e[90mb\e[39m::\e[0mInt64, \e[90mc\e[39m::\e[0mInt64\e[0m\e[1m)\e[22m\n\e[90m") | ||
| end |
There was a problem hiding this comment.
If you need a test:
julia> Val{:var"\\\""}
Val{Symbol("\\\"")}
47e69fa to
b481dec
Compare
Co-authored-by: Tim Holy <tim.holy@gmail.com>
JeffBezanson
force-pushed
the
jb/depthlimitty
branch
from
b481dec to
a7afc23
Compare
This method causes horrible invalidations and causes havoc in loading times and compilation time after Tracker.jl is loaded. For example, loading Enzyme.jl goes from 0.5 seconds to 22 seconds if Tracker.jl is loaded first. The printing of types should hopefully have been improved enough in JuliaLang/julia#49795 that this is not needed.
|
How can users use this non-interactively? |
This is an alternate implementation of #49231. It's the same idea but I implemented it just by traversing the output string, counting curly braces, and omitting text at the needed nesting depth. It handles escape sequences by skipping over them. @timholy can you poke any holes in this? I think it should work in practice and is much less code. If no fatal problems are found I'll port over the tests.