include ClearStacktrace.jl in Base

[jkrumbiegel]

cf. issue #36026

The base functionality works. I added handling of stackoverflow traces, too.

I have not dealt with any tests, yet. Also, somebody in the know should check if my methods of retrieving different parts of information about each frame are valid.

I am also checking for three boolean ENV variables in the code, to make printing a bit more customizable. They are:

JULIA_STACKTRACE_EXPAND_BASEPATHS
JULIA_STACKTRACE_CONTRACT_USERDIR
JULIA_STACKTRACE_LINEBREAKS

[JeffBezanson]

Thanks!

This is also going to need the code for handling keyword arguments in show_spec_linfo in stacktraces.jl. We could merge the new code with the show method for StackFrame, to avoid having two different ways to print the same information.

[jkrumbiegel]

This is also going to need the code for handling keyword arguments in show_spec_linfo in stacktraces.jl.

I have followed your other suggestions, but I'm not quite sure what you mean by this. How should keyword arguments be handled? They do not appear in the specialization types because methods are not specialized for them, should we still print their names and the types of the passed arguments?

We could merge the new code with the show method for StackFrame, to avoid having two different ways to print the same information.

Should I replace the show method with what I have? In what contexts is that other function used, I'm not sure what use cases I have to think about when merging.

[KristofferC]

There is a bunch of code in the StackFrame module that is only relevant for the previous stacktrace printing, for example:

julia/base/stacktraces.jl

Lines 219 to 222 in 855a08b

	color = get(io, :color, false) && get(io, :backtrace, false) ?
	Base.stackframe_function_color() :
	:nothing
	printstyled(io, Base.demangle_function_name(string(frame.func)), color=color)

So either that should be removed or the code here should replace it.

[KristofferC]

Maybe submodules should have the same color as the parent module?

[jkrumbiegel]

Maybe submodules should have the same color as the parent module?

Can submodules be so different that it would make sense to distinguish them? Otherwise that's a good idea. I don't know by what logic they are usually used, I don't really use submodules.

Another thing is that I had a heuristic before to find a module for inlined functions by copying that of another non-inlined function from the same file. But I guess that could also produce the wrong result. Is there a failsafe way to do this?

[KristofferC]

Can submodules be so different that it would make sense to distinguish them?

In many cases it is just an implementation detail of the package so I think it makes sense to keep them the same color.

[JeffBezanson]

How should keyword arguments be handled?

The current printing is:

julia> f(x; kwarg=1) = error();

julia> f(1,kwarg=3)
ERROR: 
Stacktrace:
 [1] error() at ./error.jl:42
 [2] f(::Int64; kwarg::Int64) at ./REPL[4]:1
 [3] top-level scope at REPL[10]:1

The magic is done starting at

julia/base/stacktraces.jl

Line 228 in 095e92d

if def.nkw > 0

[jkrumbiegel]

I have added keyword printing as well, but I couldn't just copy paste the code from stacktraces.jl. It's not really documented what many of the small operations do that seem to be there for edge cases. This is probably as far as I can take it because my knowledge of the internals is very limited.

[simeonschaub]

Just mentioning it here: Once #35556 is merged the global LAST_SHOWN_LINE_INFOS should be replaced by get(io, :LAST_SHOWN_LINE_INFOS, Tuple{String, Int}[]).

[tk3369]

JULIA_STACKTRACE_EXPAND_BASEPATHS
JULIA_STACKTRACE_CONTRACT_USERDIR

Do we really want opposite directions here (EXPAND and CONTRACT)? It would be more consistent to me if they're both EXPAND or both CONTRACT. Just a thought.

[jkrumbiegel]

Well the reason is that by default the user directory is part of the file path, so it has to be explicitly contracted. And the base files are just file names, so their file paths have to be explicitly expanded :) Those are the different behaviors compared to the current state

[jkrumbiegel]

I guess if we said that the new default is to print base file paths completely, then the option could be named contract base paths to get the old behavior. It's a bit annoying that those paths tend to be super long but I personally love being able to click them in VSCode. The options are meant so everybody can decide how terse or verbose they want their traces. To me for example, even though a short stack trace can be nice without line breaks, after a certain length I don't care about the overall trace size anymore, it's all about being able to visually parse it at all

[StefanKarpinski]

Seems clearer to me if they both are expand or contract but have different defaults.

[jkrumbiegel]

To proceed, I need help with the following:

• what kind of tests do we need?
• is the positional / keyword argument name functionality correctly implemented?
• is the printing of anonymous and keyword shim function names correct (all the ## business)?
• are there edge cases that I didn't handle?

[KristofferC]

I took the liberty of pushing a commit to this branch. Feel free to revert / make whatever changes you want on top of it. The changes are:

• Fixed a bunch of tests that this broke.
• Added a test for the new printing
• Rebased on master
• Made it so that submodules of Main are considered distinct which seems reasonable to me:
  
• Changed back to Stacktrace: for the header. This is used everywhere in doctests so it seems best to keep this.
• Fixed some bugs in the limited stacktrace printing
• Fixed bug with the stacktrace methodloc updater
• Fixed not printing ANSI codes (for underlined text) when the IO doesn't have the :color attribute.
• Implemented this by doing the formatting in the stackframe printer. This meant that a lot of code could be removed since almost everything just piggy-backs on that. It also fixes some bugs in corner cases (like the name in functions with keywords).

I also changed some of the defaults from some experience using this (again feel free to revert). These are:

• Changed to no line break by default. I noticed that it was very frequent that I had to scroll up to see the error message now.
• Added back the indent to the stacktrace number (similar to how it is right now)
• Changed the order of the colors so that red doesn't show so quickly since it is a bit confusing to show more red text since that is the color of the initial ERROR:.

[fredrikekre]

Is is just me that thinks it is a bit strange that , and :: stands out more than types in the signatures:

And also, argument names are bold, but isn't the type more interesting in general? I don't care what name someone happened to used in a method.

[jkrumbiegel]

I think two factors make this look a bit worse in your case. First, the contrast between white and gray is very strong, and the contrast between gray and background very low (almost unreadable). That makes the types look unimportant and the colons very important. It could be different again with another color theme.

The other thing is that your signatures are super short. The colons are supposed to be a bit more visible because they give you a visual anchor to where types / arguments begin when you have some long ones in there. It doesn't work the other way around because lower contrast is not salient between higher contrast.

[jkrumbiegel]

it's of course impossible to make the defaults perfect for every color theme, but we could put in more flags for people to set if they don't like specific choices. Here's an older screen shot from the other thread again that uses a different color theme. I think this looks much better

[mcabbott]

Is is just me that thinks it is a bit strange that , and :: stands out more than types in the signatures

This also seems a bit odd to me. The intention is to make it easier to spot where the n-th type starts. An alternative would be to print the outermost type normally, and its (possibly long) type parameters in the dim (bright_black?) now used for the whole type. That way useful data marks the same division.

[fredrikekre]

One idea would be to print the outermost type in the same color as the :: but still dim the inner type parameters. Something like this:

Edit: I see now that it was mentioned in #36026 (comment) but saw no further discussion about it.

[mcabbott]

I made a fork which implemented roughly this, to make it easy to try things on various terminals. Pictures here now updated to be on a dark background, and not to highlight the argument name:

Besides the colons, IMO it's not necessary to use super-bright colours for the modules, it's better not to out-shine the function names. And it would be a little nicer to have them in a rainbow-ish order, starting near the red of the error.

Edit: it also omits the colour from Base (and perhaps Core) which was also discussed at some point. Maybe it's too dim though?

[fredrikekre]

That looks like an improvement to me. Make a PR?

[KristofferC]

I think it is slightly better but it is becoming a little bit of a "color salad" where it is hard to scan for the information you want. Worth trying out though.

[mcabbott]

Any thoughts on how to un-salidify it?

Would using fewer colours help, like maybe just the first 3 above, then re-use? (Since RecipesPipeline is so much longer, it would still look different.)

Terminal contrast alters this a lot too, e.g. the light-mode screenshots (at my link) have much less distinction between normal and dim text.

Will try a PR when I get a minute.

[StefanKarpinski]

FWIW, I think the colors are fine.

[KristofferC]

FWIW, I think the colors are fine.

The current ones or the ones in the last screenshot?

[StefanKarpinski]

Last screenshot.

[mcabbott]

Not a PR yet, but here's a paste-able patch:
https://gist.github.com/mcabbott/06a89aa18a0e350a2281c80320a5b01d
Mostly like the above screenshot, but it makes Base & Core red not gray.

[jkrumbiegel]

I just installed 1.6 and noticed that the function names are not bold anymore, making them stick out less than I originally intended (especially if they are above and below colored Modules). This must have gotten lost in a rewrite? I've tried to follow to the point where functions are printed, but I couldn't find it. The new implementation seems to go quite deep into special cases, but the gist is that the function name is printed non-bold in the end. If they were all bold, you could see a nice vertical bold group for easy orientation.

See here, it's hard to see the function names.

[KristofferC]

It might have gotten lost, yes. Should be easy to put back though.

[mcabbott]

I tried here: ada29c2. (Without really understanding how they used to be made bold, before this.) Comparison picture here: #37773 (comment)

[NHDaly]

This PR broke cycle detection in stack trace printing in some cases 😮
Please see #37587 for more details.

I've tracked the breakage down to this PR, where you can see a before/after that shows the stack trace not reporting the recursion cycle:

julia> versioninfo()  # before
Julia Version 1.6.0-DEV.353
Commit 7dba98808a* (2020-07-02 01:54 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin18.7.0)
  CPU: Intel(R) Core(TM) i7-6920HQ CPU @ 2.90GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-9.0.1 (ORCJIT, skylake)

julia> foo() = foo()
foo (generic function with 1 method)

julia> foo()
ERROR: StackOverflowError:
Stacktrace:
 [1] foo() at ./REPL[2]:1 (repeats 79984 times)

julia> versioninfo()  # after
Julia Version 1.6.0-DEV.354
Commit 30b09dfd2b* (2020-07-02 08:02 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin18.7.0)
  CPU: Intel(R) Core(TM) i7-6920HQ CPU @ 2.90GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-9.0.1 (ORCJIT, skylake)

julia> foo() = foo()
foo (generic function with 1 method)

julia> foo()
ERROR: StackOverflowError:
Stacktrace:
 [1] foo()
   @ Main ./REPL[2]:1

The new printing is super pretty and I love it -- can we try to track down and fix this regression so that cycle detection still works with this new framework? :) Thanks!