Fix `use` insertion when `do` block starts with destructuring bind by pchiusano · Pull Request #5911 · unisonweb/unison

pchiusano · 2025-10-01T21:13:02Z

Previously, the pretty-printer would fail to insert a use clause at the start of a do block, if that do block began with a destructuring bind. This could sometimes lead to some terrible rendering with chains of operators all showing up with qualified names.

Here's a minimal example. There are two things called ++, a zoink.++ and foo.++. Thus, the code must disambiguate with a use clause:

a zoink.++ b = ()

a foo.++ b = "abracadabra"

example = do 
  use foo ++
  (x, y) = ("hi", "there")
  x ++ y ++ "z" ++ "a" ++ "b" ++ "c"

The use clause wasn't being used when viewing example because the do block starts with a Match, which isn't typically considered a candidate spot for use insertion. But actually, if the match happens to be a destructuring bind, putting a use clause there is fine.

Here's before and after -

Before:

After:

Testing

I added a transcript test and it passes all the existing transcript tests.

Misc

I fixed what ended up being an unrelated bug with Bytes literals causing imports of Bytes.fromList.impl.

pchiusano

Self review

pchiusano · 2025-10-02T01:49:19Z

parser-typechecker/src/Unison/Syntax/TermPrinter.hs

+    reannotateUp g t = case ABT.out t of
+      ABT.Var v -> ABT.annotatedVar (annotation t, g t) v
+      ABT.Cycle body ->
+        let body' = reannotateUp g body
+        in ABT.cycle' (annotation t, snd (annotation body')) body'
+      ABT.Abs v body ->
+        let body' = reannotateUp g body
+        in ABT.abs' (annotation t, snd (annotation body')) v body'
+      ABT.Tm body ->
+        -- literals like 0xsaaff don't contribute to the annotations
+        -- even though they desugar to function calls
+        let body' = reannotateUp g <$> body
+            ann = if isLiteral t then mempty else g t <> foldMap (snd . annotation) body'
+        in ABT.tm' (annotation t, ann) body'


This function didn't quite exist in ABT and it's too special purpose to add there since it's specific to the Unison term functor - nodes that are bytes literals have their annotation zero'd out to avoid contributing to use clauses that are inserted higher in the syntax tree.

pchiusano · 2025-10-02T01:49:58Z

parser-typechecker/src/Unison/Syntax/TermPrinter.hs

-pattern Bytes' :: [Word64] -> Term3 v PrintAnnotation
+pattern Bytes' :: [Word64] -> Term2 v at ap v a
 pattern Bytes' bs <- (toBytes -> Just bs)

-toBytes :: Term3 v PrintAnnotation -> Maybe [Word64]
+toBytes :: Term2 v at ap v a -> Maybe [Word64]


Needed to generalize these signatures to get it to typecheck. They were needlessly monomorphic.

pchiusano · 2025-10-02T01:51:28Z

parser-typechecker/src/Unison/Syntax/TermPrinter.hs

-          Delay' x
-            | Match' _ _ <- x -> do
+          Delay' x@(Match' scrutinee cs)
+            | not (isDestructuringBind scrutinee cs) -> do
                px <- pretty0 (ac Annotation Block im doc) x
                let hang = if isSoftHangable x then PP.softHang else PP.hang
                pure . paren (p > Control) $
                  fmt S.ControlKeyword "do" `hang` px
-            | otherwise -> do
-                let (im0', uses0) = calcImports im x
-                let allowUses = isLet x || (p == Bottom)
-                let im' = if allowUses then im0' else im
-                let uses = if allowUses then uses0 else []
-                let soft = isSoftHangable x && null uses && p < Annotation
-                let hang = if soft then PP.softHang else PP.hang
-                px <- pretty0 (ac Annotation Block im' doc) x
-                -- this makes sure we get proper indentation if `px` spills onto
-                -- multiple lines, since `do` introduces layout block
-                let indent = PP.Width (if soft then 2 else 0) + (if soft && p < Application then 1 else 0)
-                pure . paren (p > Control) $
-                  fmt S.ControlKeyword "do" `hang` PP.lines (uses <> [PP.indentNAfterNewline indent px])
+          Delay' x -> do
+            let (im0', uses0) = calcImports im x
+            let allowUses = isLet x || (p == Bottom) || isDestructure x
+                  where
+                    isDestructure (Match' scrutinee cs) = isDestructuringBind scrutinee cs
+                    isDestructure _ = False
+            let im' = if allowUses then im0' else im
+            let uses = if allowUses then uses0 else []
+            let soft = isSoftHangable x && null uses && p < Annotation
+            let hang = if soft then PP.softHang else PP.hang
+            px <- pretty0 (ac Annotation Block im' doc) x
+            -- this makes sure we get proper indentation if `px` spills onto
+            -- multiple lines, since `do` introduces layout block
+            let indent = PP.Width (if soft then 2 else 0) + (if soft && p < Application then 1 else 0)
+            pure . paren (p > Control) $
+              fmt S.ControlKeyword "do" `hang` PP.lines (uses <> [PP.indentNAfterNewline indent px])


This logic is finnicky but it is the same as before, except that we allow use insertion if the first thing in a do is a destructuring bind.

pchiusano and others added 8 commits October 1, 2025 16:08

fix use insertion when do block starts with a destructuring bind

4f57606

automatically run ormolu

f001266

add transcript

cf8ad66

revert isLet function in favor of more specialized logic

0a4a5b5

automatically run ormolu

c196a00

simplify logic

1c3e28f

don't emit Bytes.fromList.impl imports

d283094

automatically run ormolu

8c1ca63

pchiusano commented Oct 2, 2025

View reviewed changes

pchiusano requested a review from runarorama October 2, 2025 01:52

pchiusano added 2 commits October 1, 2025 20:55

refresh CI

8f74e5a

refresh round trip transcripts

0ef0da7

runarorama approved these changes Oct 2, 2025

View reviewed changes

runarorama merged commit 9c5d297 into trunk Oct 2, 2025
31 checks passed

runarorama deleted the fix/use-insertion branch October 2, 2025 11:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `use` insertion when `do` block starts with destructuring bind#5911

Fix `use` insertion when `do` block starts with destructuring bind#5911
runarorama merged 10 commits intotrunkfrom
fix/use-insertion

pchiusano commented Oct 1, 2025 •

edited

Loading

Uh oh!

pchiusano left a comment

Uh oh!

pchiusano Oct 2, 2025

Uh oh!

pchiusano Oct 2, 2025

Uh oh!

pchiusano Oct 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pchiusano commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Misc

Uh oh!

pchiusano left a comment

Choose a reason for hiding this comment

Uh oh!

pchiusano Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

pchiusano Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

pchiusano Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pchiusano commented Oct 1, 2025 •

edited

Loading