docs: close out the GHC-style Note pass (#44)#130
Merged
Conversation
Add Note [IR is assumed well-typed] (IR.Optimizer), cited from constantFolding and betaReduce, and make the conscious call on the remaining low-priority #44 candidates (left as adequate inline comments, or deliberately not retitled in the upstream-adapted CoreFn.Laziness). Comments only; generated code unchanged. Closes #44.
There was a problem hiding this comment.
Pull request overview
This PR completes the final documentation pass for issue #44 by adding a single remaining low-priority GHC-style Note that documents a load-bearing invariant in the IR optimizer, with citations from the rewrite sites that depend on it. The changes are comment/documentation-only and do not modify compiler behavior or generated output.
Changes:
- Added
Note [IR is assumed well-typed]to document the optimizer’s reliance on upstream type checking. - Updated the relevant rewrite sites (
constantFoldingandbetaReduce) to cite the new Note. - Added a
changelog.d/fragment recording the closure of the #44 documentation pass.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/Language/PureScript/Backend/IR/Optimizer.hs | Adds and cites a new GHC-style Note documenting the “IR is well-typed” optimizer invariant. |
| changelog.d/20260625_203341_unisay_notes_low_items.md | Records the documentation-only change and closure of #44. |
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
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.
Summary
The final #44 batch. It adds the one remaining low-priority Note that earns its keep and makes a conscious call on the rest, so the issue's "decide consciously rather than by omission" goal is met for every listed item. Comments only, so generated code, goldens, and eval oracles are unchanged.
Closes #44.
Note [IR is assumed well-typed]inIR.Optimizer. The optimizer trusts the type checker rather than re-checking:constantFoldingrewritesEq True btobbecausebmust be aBool, andbetaReducesubstitutes an argument for a parameter without checking the types match. Cited from both rewrites.Conscious calls on the rest
Left as adequate inline documentation (a titled Note would add ceremony without a second citation site):
InternalIdentData(Names) already has a complete Haddock.Backend.IR.mkImports) has a one-line comment explaining the always-add-then-DCE approach; it is a single site.unsafePerformIOinPSString.decodeStringalready carries the standard safety argument.Deliberately not retitled, to avoid divergence from upstream
purs:CoreFn.Lazinessprose (the full delay/force rules, the USE-INIT/USE-USE/USE-IMMEDIATE derivation, and theMaxRoseTreetermination argument) is thorough and only cited from within that file. Batch 3 already gave the cross-module contract (the runtime-lazy convention) and the transform a citable anchor; titling the rest would diverge from the adapted upstream source for marginal benefit.Note discovered along the way
@inline neverdoes not currently veto inlining (documented inNote [Inline annotations and inlining heuristics]from the previous batch):isInlinableExprtreatsJust NeverlikeNothing, so a ref / small literal / single-use binding is still inlined. Flagging it here; whethernevershould actually veto is a behavior question for a separate change, not this docs pass.Checklist
changelog.d/fragment for any user-facing change (scriv createin the dev shell), or this change ships nothing releasable (CI, docs, or an
internal refactor).
nix develop),fourmolu -i lib/ exe/ test/andhlint lib/ exe/ test/are clean.cabal test allpasses; structural goldens werere-accepted on purpose if codegen moved (
PSLUA_GOLDEN_ACCEPT=1), andeval/golden.txtstill holds.