For analysing evidence uses we collect evidence uses in,
```haskell
requestedEvidence :: Map Declaration (Set Name)
```
In analyseEvidenceUses, we loop over all the names in all the sets of
the map, to construct dependency graph after calling `getEvidenceTree`
on the name. However, these names in sets across different
declarations are duplicated a lot. In one example in a repo at work,
we have 16961625 names in which only 200330 are unique. So now, we
instead pre-construct an evidence trees map `Map Name [Declaration]`
for all the unique name and perform a lookup in this map to construct
the graph.
In a private repo, the times before this change and after
```
❯ find . -name '*.hie' | wc -l
1097
❯ time result/bin/weeder # weeder from master
real 5m53.707s
user 5m50.350s
sys 0m2.206s
❯ time result/bin/weeder # weeder from this branch
real 0m34.008s
user 0m31.716s
sys 0m2.196s
```
This causes a directory listing of `./.` to be forced (to find all `.hs` files), which for my checkout is a huge search. We don't really need this check though, so for tests it can be turned off. For me, this brings test execution time all the way down to <0.1s
* Rewrite tests to use `tasty` & `tasty-golden`
* Sort weeds by column too
---------
Co-authored-by: Ollie Charles <ollie@ocharles.org.uk>
Co-authored-by: Tom Sydney Kerckhove <syd@cs-syd.eu>
This keeps the general shape of the test suite, but instead of using HSpec as the driver we now use tasty. The main benefit here is we can use `tasty-golden`, which makes it easier to update the golden files if the output legitimately changes.
* outputableDeclarations optimisation
We now only match on declarations that can appear in the output (i.e.
with a span) when evaluating `roots` and `dead` in `runWeeder`,
instead of searching through all of them.
* initialGraph optimisation
`addAllDeclarations` turned out to largely be an unnecessary step:
`define` and `addDependency` already created vertices for the
involved declarations on their own.
This skips that step when unused-types = false, and otherwise
minimises the number of added vertices.
* Compile regex in Config.hs
GHC does not optimise repeated calls to `=~`, so we were
recompiling the regex for every single declaration we matched on.
This took up 15% of runtime and 40% of memory usage.
This compiles regular expressions in advance, immediately after we
parse the config file. Besides performance, this has the additional
benefit of making `runWeeder` a total function: regex parse
failures are now emitted alongside TOML errors.
* Parallel analysis
We now run most of our analysis on each `HieFile` in parallel.
Doing this in batches of 100 turned out to not have any performance
difference from doing this with 1 `HieFile` at a time.
Given enough cores, this can almost double Weeder's performance.
* getHieFiles as a stream
This uses lazy IO via `getChanContents` to begin analysis in
`runWeeder` before `[HieFile]` is fully read.
This neither benefits nor harms performance when Weeder is given
only one core, but improves performance by about the runtime of
`getHieFiles` when given two or more cores. On my machine this is
15%, but this may vary depending on disk speed.
As following uses of type class instances requires information from
all HIE files, it is now done in a separate stage via
`analyseEvidenceUses`, applied only after the initial analysis is
fully evaluated.
* Fix throwing exit code 2 in getHieFiles
Switches `forkIO` to `async` in order to rethrow exceptions
encountered by `getHieFiles`.
* Replace type families with lists
* Set capabilities with -j and -N
* Update comments in Weeder.hs
---------
Co-authored-by: Ollie Charles <ollie@ocharles.org.uk>
* Use separate exit codes
* Update Spec.hs
* Use exit code 3 for config parse error
* Update src/Weeder/Main.hs
Co-authored-by: Ollie Charles <ollie@ocharles.org.uk>
* Replace Show instance with formatWeed
* Add constants for exit codes
* Exit code 4 when no hie files found
* Replace .hie with HIE in error message
* Override unexpected exit codes
* Put weeder exceptions in a datatype
* Add mainWithConfig'
Done in order to not expose WeederException while still exporting mainWithConfig
* Do not intercept ExitCode
* Move Weeder to Weeder.Analysis
* Move runWeeder to Weeder
* Revert "Move runWeeder to Weeder"
This reverts commit f3bd25f25c.
* Revert "Move Weeder to Weeder.Analysis"
This reverts commit e84e790735.
* Move runWeeder to Weeder.Run
* Redundant imports
* Change Weeder.Main to Weeder.Run in Spec.hs
* Wrap getHieFiles with handleWeederException
* Remove Weeder.Run exports from Weeder.Main exports
---------
Co-authored-by: Ollie Charles <ollie@ocharles.org.uk>
* Tests for datatypes in output
The .stdout file for TypeFamilies is a concept for what the output of a type family instance weed may look like.
* Assign spans to data declarations
* Support for record field occurrences
* Lookup types of declarations in addAllDeclarations
* Analyse type synonyms and type family declarations
* Add unused-types config field
* Fix warning in analyseExport
* Add annsContain function
* Fix explicit foralls
* Type alias GADT test
* Add type signatures to topLevelAnalysis
* Fix false positives with data declarations
* Keep acyclicity in record declarations
* Optional weeder.toml and --write-default-config
* Add prop_configToToml to tests
* Add module to rootInstances and rootClasses
* Specify modules in root-classes and root-instances
* Add ConfigInstanceModules test
* Make instance and class fields optional
* Add IsString and IsList to default root-classes
* Move prop_configToToml to tests
* Mandatory config file
* Introduce PatternWithModule type
* Introduce CLIArguments type
* Lowercase language pragma
* Move main in test suite back to Spec.hs
* Add --no-default-fields flag
* Remove double addition of declarations
* Implement test suite
* Add mainWithConfig'
* Export dotfiles of dependency graphs
* Draw dotfiles to PNG via graphviz
* Flag for drawing graph PNGs
* Type class instances have spans and dependencies
Also includes class declarations and (both regular and standalone) derived instances. Consequently, they show up in the output if unreachable.
* Follow type class evidence uses back to bindings
* Add failing tests (number and string literals)
* Failing test for OverloadedLists
* More tests, not all failing
Also tested on 9.4.5.
`OverloadedStringsNoSig` correctly gives no output. `Monads` incorrectly claims that the instances for `Identity'` (used by `bar`) are unreachable, while correctly giving no output on the instances for `Identity` (used by `foo`). `RangeEnum` incorrectly claims that `$fEnumColour` is unreachable.
Note that what all the incorrect outputs have in common are top-level type signatures. The type signature does not have to be immediately relevant: see `OverloadedStringsNoSig` - the type of `root` and `root'` is `Char`, but explicitly writing that breaks their evidence variables for `IsString`. When type signatures are omitted, evidence usages for syntax are present in `hie` files as expected.
In `Monads.hie`, this is the node corresponding to `bar`:
```
929 │ Node@test/Spec/Monads/Monads.hs:(36,1)-(38,13): Source: From source
930 │ {(annotations: {(FunBind, HsBindLR),
931 │ (Match, Match),
932 │ (XHsBindsLR, HsBindLR)}),
933 │ (types: [304]), (identifier info: {})}
```
and this is the node for `foo`:
```
837 │ Node@test/Spec/Monads/Monads.hs:(31,1)-(33,12): Source: From source
838 │ {(annotations: {(FunBind, HsBindLR),
839 │ (Match, Match),
840 │ (XHsBindsLR, HsBindLR)}),
841 │ (types: [302]),
842 │ (identifier info: {(name $dNum, Details: Just 3 {usage of evidence variable}),
843 │ (name $fMonadIdentity, Details: Just 167 {usage of evidence variable}),
844 │ (name $dMonad, Details: Just 167 {evidence variable bound by a let, depending on: [$fMonadIdentity]
845 │ with scope: LocalScope test/Spec/Monads/Monads.hs:(31,1)-(33,12)
846 │ bound at: test/Spec/Monads/Monads.hs:(31,1)-(33,12)}),
847 │ (name $dNum, Details: Just 3 {evidence variable bound by a let, depending on: [$dNum]
848 │ with scope: LocalScope test/Spec/Monads/Monads.hs:(31,1)-(33,12)
849 │ bound at: test/Spec/Monads/Monads.hs:(31,1)-(33,12)}),
850 │ (name $dNum, Details: Just 3 {evidence variable bound by a let, depending on: [$dNum]
851 │ with scope: LocalScope test/Spec/Monads/Monads.hs:(31,1)-(33,12)
852 │ bound at: test/Spec/Monads/Monads.hs:(31,1)-(33,12)})})}
```
The appearance of `foo` and `bar` in GHC's renamer AST does not differ in any meaningful way. In the typechecker AST, `bar` contains a (seemingly redundant) `EpAnn` annotation, which is missing in `foo`, but is otherwise the same everywhere else. For this reason I suppose the problem is not too deep within GHC and any required fixes are probably limited to modules directly handling `hie` files.
* Mark tests as failing
A `.failing` file should contain the current expected output of a failing test, overriding the `.stdout` file. This allows the test suite return a 0 exit code without having to disable failing tests.
If the test happens to return exactly the correct output contained in the `.stdout` file, it fails with a `not expected:` failure.
I think it would be worthwhile to make a small extension of `hspec` that allows marking tests as expected failures in a more polished way, assuming such an extension does not already exist.
* Tests for OverloadedLabels and ApplicativeDo
OverloadedLabels already works perfectly, but ApplicativeDo has the same problem as Monad
* Add InstanceRoot constructor
* Store pretty-printed type in InstanceRoot
* Add root-instances and root-classes fields
* Clean up tests
* Show pretty-printed type of instances in output
* Omit instance OccNames in output
* MonadReader for pretty-printed instance types
* MonadReader for following evidence uses
* Update test/Spec/InstanceRootConstraint.toml
Co-authored-by: Ollie Charles <ollie@ocharles.org.uk>
---------
Co-authored-by: Ollie Charles <ollie@ocharles.org.uk>
* Implement test suite
* Add mainWithConfig'
* Export dotfiles of dependency graphs
* Draw dotfiles to PNG via graphviz
* Flag for drawing graph PNGs
* Remove failing tests for now
* Combine mainWithConfig and mainWithConfig'