Custom Debug Visualisations #120
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We used to, for "boring tys" - in `obtainTerm`, use `depth = maxBound` to traverse arbitrarily deep - right after, in `termToVarInfo`, use `deepseqTerm` to force it till the end again. To simplify, always do the latter and forget the former. In preparation of #47
This commit introduces the haskell-view-debugger package with a class for defining custom visualisations for the debugger It refactors and improves the internals of the debugger variable inspection code to leverage custom instances when they are available. We do this by compiling on the fly the method applied to the dictionary for the type of the value we're inspecting, loading it, and applying it to the value. Lastly, we have special logic to decode the return types (VarValue and VarFields) from their heap-representation Terms. We get rid of the ad-hoc special logic for displaying strings and/or boring types and for seqing, resulting in a more uniform and extensible design. This commit does not include two things that will follow up: 1. Caching DebugViewInstances with a Core.Map.Type trie map 2. Loading the DebugView class module and built-in instances ourselves when the user program does not transitively depend on them. Fixes #47 Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com>
Fixes bug that caused a crash when inspecting a non-custom-debug view value inside of a custom-debug-view value
Includes test for non-debug-view value inside of debug-view value instance
Rather than pprTraceM
We haven't needed these in a long while since the adapter and debugger lib run in the same process
Continues #47 and adds test for this
Makes sure to try the following in order - First, try to find existing haskell-debugger-view dependency - Second, try to load built-in haskell-debugger-view modules if no dependency was found - Third, gracefully default to not using custom instances at all if the built-in modules failed to load Things left to do: - gracefully try to load modules for the orphan instances (text, bs, containers, ...) both for in-memory and as dependency. - refactor to cache DebugInstanceView - refactor Monad module to clean this up a bit further
Otherwise the built-in class being loaded doesn't work. All tests now pass again at this point
This reverts commit c406372. In the future, we may want to guard all dependencies behind cabal flags that the user can tweak when depending on @haskell-debugger-view@. However, it seems that currently, hie-bios ignores the cabal flags given in the cabal.project. Therefore, it's hard to test so we'll postpone this a bit.
Ensures that we load the orphan instances for GHC.Debugger.View.Containers, but not yet for the in-memory case.
Makes the debugger try to load all additional modules which provide orphan instances, even when the haskell-debugger-view package is not part of the transitive closure. We try to load every extra module independently, ignoring if it doesn't compile.
And now it works both when containers is and isn't available.
And it still uses the custom display. Plus a few cleanups
Plus a few cleanups and improvements. And fix the testsuite existing output. And a new test for `text`
alt-romes
force-pushed
the
wip/romes/47
branch
6 times, most recently
from
5504f44 to
f933f75
Compare
This ensures that `cabal sdist` includes all files necessary for compilation in the tarball. Otherwise, these files try to be included by TH but wouldn't be available.
This ensures we ignore certain logs according to the verbosity given by the user
It turns out it was too noisy to prefix every GHC log (like [1 of 1] Compiling ...) with time and severity information from the `Recorder`. Let's keep it as was for now.
We try to load GHC.Debugger.View.Class and all the orphan instances modules whenever the package is not part of the transitive closure. While doing this, we print a lot of compilation information which is irrelevant to the user. With this commit, we'll hide it by default. It is still printed out with -v3.
alt-romes
force-pushed
the
wip/romes/47
branch
from
f933f75 to
32ca05e
Compare
Fixes the step-out test. Why was it broken? Step-out with do-notation relies on optimisations to inline the definition of >>= (at least for now). However, we now loading GHC.Debugger.View.Class in addition to the user program. Even if the user specified -O1 in their extra GHC args, we would load GHC.Debugger.View.Class without optimisations (since it didn't need them). The issue is that the module interface containing >>= would be stripped of all pragmas and unfoldings because GHC.Debugger.View.Class didn't want them. In turn, when the user Main module was loaded, >>= would not have the unfoldings the user desired. Thus, step-out didn't work. The fix is to include -fno-ignore-interface-pragmas to make sure we never discard the interface additional information, and also -fno-unoptimized-core-for-interpreter to guarantee this isn't undone by the interpreter not allowing optimisations by default. Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com>
This reverts commit 3093efa. Unfortunately, setting `-fno-omit-interface-pragmas` changes the optimisation behavior of the program because, even if we use -O0, the existence of more unfoldings gets used by the compiler when simplifying. This makes debugging more brittle since, e.g., more newtype expressions get inlined and further away from the original program (breaking tests such as the one for newtype variable inspection)
Make sure to parse and use the extraGhcArgs as dynflags which are shared across all units. Failing to do this caused a bug with step-out where `>>=` from base was being loaded without interface pragmas because GHC.Debugger.View.Class was being compiled with -O0 while the user program was meant to be compiled with -O1 (extraGhcArgs = -O -fno-unoptimized-core-for-interpreter). We want to use the same optimization level across all units, and, for correctness, pass the extra args to all units. See also the two previous commits for an alternative way to fix this particular bug, but with downsides of their own and less principled. Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com>
alt-romes
force-pushed
the
wip/romes/47
branch
from
064f0a6 to
0d18ff4
Compare
Collaborator
Author
|
Finally nailed it with the help of @mpickering Thanks for your initial contribution @meipp. Turns out it wasn't so simple after all! (at all...!) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduces the ability to write custom debug visualisations for datatypes and refactors the internals to use the built-in provided instances for datatypes like Strings.
Fixes #47