Trim and consolidate the codebase; move to GHC 9.10.3 - #150
Merged
sfultong merged 18 commits intoAug 27, 2026
Merged
Conversation
Verified per component by matching imports against build-depends; every component now builds clean with -Wunused-packages enabled. Also removes the unused data-files stanza (nothing references Paths_telomare), the vestigial Setup.hs (build-type: Simple), commented-out cruft, and a duplicate .gitignore entry. cabal-version bumped to 3.4.
Every deleted definition was verified to have zero call sites across src/, app/, and test/ (including implicit module exports). Highlights: the unused pure twins of the super*/indexed* step family in Machine.hs (superStepM's decendant-based filters are the live semantics), getSizesM (an uncalled near-copy of sizeTermM), the no-op removeChecks, the stale unitTests_ duplicate in Spec.hs, and the unreachable TestIExpr/Arbitrary-UPT apparatus in test/Common.hs.
The format-lint logic existed four times (twice in flake apps, once in checks, once in CI shell). Now one app script with a --check mode backs apps.format and apps.format-lint, and CI relies on nix flake check's format-lint check instead of reimplementing it. hie.yaml previously mapped all five test suites to path "test", so only the first cradle could match; each test file now maps to its own component. The GHC package set is factored into one hsPkgs binding.
- Telomare.Util (debugTrace, padRight, plural) replaces six per-module copies - Telomare.Lexical shares lexical facts between the parser and the LSP lexer - topologicalSort lifted to one definition; PrettyBasic replaces the two pretty-printer newtypes; truncateToData hoisted into Machine - Fast codecs bridge IR.Base's s2b/b2s instead of reimplementing them - evalLoop variants fold through evalLoopCore; Main.hs scans imports with the real parser; LSP lexes block comments and drops its drifted keyword list - purely recovers pure step functions from their monadic twins (basicStep, abortStep); traverseScoped/patternBinders replace five hand-rolled scope-tracking walks across Resolve, Desugar, and LSP - ConformanceTests asserts the sized, metered, and fast evaluators agree on every fixture that sizes, replacing the scattered one-off agreement tests - Test suites share their eval harness and prelude loading; unused build-depends pruned
The nix build does not compile the working tree: haskell-flake feeds it a `cabal sdist` tarball, and a tarball contains only what the .cabal file declares. Dropping `data-files` (fec0336, on the grounds that `Paths_telomare` is never imported) was right about the module and wrong about the effect -- it also removed the only declaration that put Prelude.tel and the other programs into that tarball, so every test suite inside the nix sandbox failed on `Prelude.tel: openFile: does not exist`. Declare them as `extra-source-files` instead, which is what they actually are: inputs the build and the tests read, not data to install. Globs keep the list from drifting as programs are added.
Update every flake input (nixpkgs, flake-parts, haskell-flake, flake-compat) and point the single `hsPkgs` binding at ghc910, which is now nixpkgs' default curated package set -- so package coverage and HLS support are the best on offer rather than a set kept alive for us. cabal-version goes to 3.12, not further. cabal-install is 3.16, but the nix build compiles Setup.hs against the Cabal bundled with GHC (3.12.1.0), and that parser rejects 3.14 outright. 3.12 is the highest version both paths accept. GHC 9.10 puts -Wx-partial in -Wall, so the zero-warning build needed the partial functions gone. Each one had a guard next to it already proving the list was non-empty; saying so in a pattern instead is both total and shorter. The lexer's three `null cs`/`head cs` pairs collapse into a `startsWith` predicate. foldl' now comes from Prelude. The newer hlint also asks that a pass ending in `($ []) . runReaderT . cata f` name its seed with flip, and that a toList foldl' does not need go away. Both applied. The copyright year moves to 2026 in both places that carry it, the cabal field and the LICENSE notice.
The Unreleased section documented the module reorganization and the -Wall pass but stopped there, so six commits of work would have landed on master unrecorded. The README's facts were already accurate -- the keyword list still matches Telomare.Lexical.reservedWords and the identifier grammar still matches identifierStart/identifierContinueChar -- but its module map did not name Lexical or Util. A module whose entire reason for existing is that two consumers read it instead of restating it is exactly what that paragraph is for.
`nix run .#push-cachix` has been failing at the point where it collects the app closures. It named them with `nix eval --raw .#apps.<system>.<name>.program` and fed the store paths to `nix path-info --recursive`, but evaluating an app only computes what its path would be; nothing builds it. `path-info` then rejects the path as invalid and the run aborts before it pushes anything. That is not a rare state. The LSP wrapper embeds `self.lastModifiedDate`, so its path changes with every commit and every dirty working tree, and a freshly named path has by definition never been built. The three shell tools now live in the `let` block and are interpolated into the script's text by name. A store path that appears in a derivation's text is one of its build inputs, so it is realised before the script can run — the failure is not handled, it is unrepresentable. `default` and `repl` drop out of the list entirely: they are binaries of the package the script already builds. Hoisting them also removes the last reason for `apps.format-lint` to reach through `self'.apps.format.program` for a path it can now name directly.
sfultong
requested changes
Aug 24, 2026
|
|
||
| debugTrace :: String -> a -> a | ||
| debugTrace s x = if debug then trace s x else x | ||
| -- |Recover a pure step function from its monadic twin by running it in |
Collaborator
There was a problem hiding this comment.
Let's leave Machine.hs alone for now, and let me clean it up later when I figure out if a non-monadic sizing approach will have a speed advantage
Contributor
Author
There was a problem hiding this comment.
Looked into a monadic and non-monadic optimization of sizing. Ended up reverting changes in Machine.hs to what master has doing some optimizations on top of that.
On fable's words:
Machine.hs is restored byte-for-byte, and I benchmarked both routes to answer the open question. Optimizing the monadic code got sizing from ~122s to ~14s on tictactoe.tel (8.7x, allocation 159 GB → 9.8 GB): the ReaderT layer was dead, the step tower and the derived Traversable instances were running unspecialised (fixed with INLINABLE/hand-written INLINE traverse), and the SizedRecursion merge needed a mempty fast path. Compiler output is byte-identical throughout and all suites pass. I also prototyped the non-monadic version: ~20x faster still, but unsound — laziness drops sizes recorded in branches whose value is never demanded (it recovers 8 of 12 recursion sites on tictactoe and under-counts 3). Making it sound means forcing eagerness at every consumption point, which is exactly what the StrictAccum monad already provides — so I'd keep the monadic route. The prototype is on my nonmonadic-sizing-prototype branch if you want to look at it.
| import Debug.Trace (trace) | ||
|
|
||
| -- | Master debug switch. Flip in a dev checkout; never commit True. | ||
| debug :: Bool |
Collaborator
There was a problem hiding this comment.
the problem with a singular debug switch, is that it will generate a lot of output, and most of it won't be relevant for debugging specific issue. The finer-grained version is better
Review feedback on Stand-In-Language#150: leave Machine.hs alone until the non-monadic sizing experiment is settled. This puts back the independent pure step bodies and the pure step family (superStep, superAbortStep, indexedAbortStep, indexedSuperStep, indexedInputStep', indexAbortIfUnboundStep, indexSwitchSuperSplitStep, indexedInputStepM', zeroedInputStepM, unsizedTestUnsized, lamB), and drops the purely wrapper and the top-level truncateToData. Knock-ons: Meter.hs keeps its local truncateToData again, and Reference.hs and Size.hs go back to Machine's debugTrace re-export instead of Telomare.Util's.
Review feedback on Stand-In-Language#150: one master debug switch floods the output with traces from every pass at once; the finer-grained version lets you turn on only the module under investigation. Size, Size.IR (primed), Resolve, TypeCheck and Driver get their local debug/debugTrace definitions back (Machine already recovered its own in the previous commit), and Telomare.Util shrinks to the uncontested string helpers.
Proposal on top of the requested reverts, motivated by measurement. sizeTermM ran its whole evaluation in ReaderT (TCallStack a) over StrictAccum, but nothing ever extends that stack (no local anywhere) and its only reader, failAndPrintStack, has no call sites: every bind at every node paid a closure layer for a constant empty environment. unsizedStepM''' loses the MonadTrans wrapper and sizeTermM runs transformNoDeferM directly in StrictAccum SizedRecursion. Sizing tictactoe.tel: 122s -> 81s elapsed, 159GB -> 89GB allocated, 260s -> 131s CPU; the compiled .telc is byte-identical and the sizing test suite passes.
Proposal on top of the requested reverts, motivated by measurement. The *StepM family, transformNoDeferM and the gate helpers are deeply overloaded (base-functor classes plus the monad dictionary), and without INLINABLE their unfoldings never cross the module boundary, so the sizing hot loop in Telomare.Size ran entirely on dictionary passing. Marking them INLINABLE lets the specialiser compile them at the concrete sizing types. Sizing tictactoe.tel: 81s -> 31s elapsed on top of the ReaderT removal (122s on master), 89GB -> 45GB allocated; .telc byte-identical; sizing and main test suites pass.
Proposal on top of the requested reverts. Almost every bind in the sizing pass merges two empty size maps; returning the non-empty side directly avoids the unionWith walk, and inlining StrictAccum's bind and ap lets those merges disappear at the specialised call sites. A small win on top of the previous two (within run-to-run noise on tictactoe, ~27-33s), kept for its principle: the hot loop should not pay for bookkeeping it almost never uses.
hhefesto
force-pushed
the
hhefesto/simplification-review
branch
from
August 27, 2026 00:52
da2244f to
4e14cb8
Compare
The derived traverse methods carry no unfoldings, so the sizing pass's monadic traversal ran them unspecialised: dictionary passing plus boxed applicative chains at every node. Profiling tictactoe.tel showed the derived BasicExprF and UnsizedExprF traversals plus StrictAccum's resulting fmap/liftA2 boxes at ~75% of runtime. Writing traverse by hand with INLINE lets it specialise at the call sites: 28.7s / 45GB allocated drops to 13.7s / 9.8GB, with byte-identical compiler output.
failAndPrintStack lost its last caller when the sizing pass dropped its ReaderT layer; remove it along with the TCallStack machinery and the MonadReader import that existed only to serve it.
HANDOFF.md is working state for in-progress rounds, not something master should carry; it stays as a local untracked file instead.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
A review-driven cleanup pass over the whole tree: −1,138 lines net (690 added,
1,828 removed, 44 files). No behaviour change is intended anywhere, with the exception
of four bugs fixed along the way, called out below. Eight commits, each self-contained
and reviewable on its own.
Commits
fec0336d70b383d4cb750560a393354c385ebd3825c8ded21e97b6d9Bugs found while doing it
The nix build had not compiled the tests since
data-fileswas dropped.haskell-flake builds from a
cabal sdisttarball, not the working tree, and a tarballcontains only what the
.cabalfile declares. Removingdata-files— correct on itsface, since
Paths_telomareis never imported — also removed the only thing puttingPrelude.teland the other programs into that tarball, so every suite inside the nixsandbox died on
Prelude.tel: openFile: does not exist. They are nowextra-source-fileswith globs, which is what they actually are: inputs the build andthe tests read, not data to install.
nix flake checkgenuinely runs the suites again.topologicalSortexisted twice inResolve.hs, and the two copies returned oppositeorders. Lifted to one definition; the
letsToAppscall site keeps its order with anexplicit
reverse.nix run .#push-cachixcould not finish. It named the app closures withnix eval --raw .#apps.<system>.<name>.programand handed the paths tonix path-info --recursive, but evaluating an app only computes what its path would be; nothingbuilds it, and
path-inforejects an unbuilt path. The LSP wrapper embedsself.lastModifiedDate, so its path changes with every commit — it is essentially neveralready built. The three shell tools are now named by interpolation into the script's
text, which makes them build inputs of it, so they are realised before it can run.
Three drifts between the LSP lexer and the real parser:
wherewas highlighted as akeyword though the language does not reserve it, identifiers could only start with an
ASCII letter, and
{- -}block comments were never lexed at all. All three came from thelexer restating parser facts. The facts now live in one module both read.
Abstractions introduced
Telomare.Util—debug/debugTracehad six copies (one module had tohidingthe name and rename it
debug');padRight/pluralhad four and two.Telomare.Lexical— the reserved words, identifier character classes and commentdelimiters, shared by
Telomare.Parseand the LSP lexer so they cannot drift again.traverseScopedinIR/Surface.hs— one binding-aware traversal of the surfacefunctor, replacing five hand-written scope walks (
Resolve.qualifyTerm,Desugar.optimizeBuiltinFunctions, and three in the LSP).mapScopedandfoldMapScopedfall out of it viaIdentityandConst.purelyinMachine.hs— the step functions were written twice, once pure and oncemonadic, differing only in
pure/liftM2. The monadic body is now the only body;basicStep = purely basicStepM. Pure signatures are unchanged.test/ConformanceTests.hs— the compiler has three ways to run a program (sizedinterpreter, step meter, unsized fast runtime). A corpus-driven spec asserts all three
agree, replacing scattered one-off comparisons.
Toolchain
GHC 9.6.7 → 9.10.3 (nixpkgs' default curated set), all four flake inputs updated,
cabal-version3.4 → 3.12.3.12 is a measured ceiling, not a guess: cabal-install is 3.16 and would accept more, but
nix compiles
Setup.hsagainst the Cabal bundled with GHC (3.12.1.0), whose parserrejects 3.14 outright. GHC 9.10 also adds
-Wx-partialto-Wall; everyhead/tailit flagged already had a guard next to it proving the list non-empty, so each became a
pattern match instead.
Deliberately not done
Four consolidations that looked available and are not, each verified rather than assumed:
steps: it diverges on sized recursion (forced gate branches) and inflates counts. The
fast runtime's flat ADTs are its entire purpose. The conformance suite is the right
guard here, not a shared implementation.
stuckStep/transformNoDeferkeep their twins — the monadic bodies needTraversable f, whichbasicEvalinEval/Reference.hscannot supply.indexedInputStep/Mare not twins at all: the monadic one has three extraAnyFcases, a live semantic difference.
Levels.walkis not migrated totraverseScoped— it needs the right-hand-side terms,two maps and a depth-varying
App.Verification
nix develop -c cabal build all— zero warnings, every component on-Wall -Wunused-packagesnix flake check— all checks passed (package build incl. in-sandbox suites, plusformat/lint)