Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How does the pretty printer decide to qualify names? #5490

Open
SystemFw opened this issue Dec 9, 2024 · 2 comments
Open

How does the pretty printer decide to qualify names? #5490

SystemFw opened this issue Dec 9, 2024 · 2 comments
Labels
error-message Request for improved error message

Comments

@SystemFw
Copy link
Contributor

SystemFw commented Dec 9, 2024

How do we explain the behaviour of the pretty printer in this doc?
https://share.unison-lang.org/@systemfw/aoc-2024/code/main/latest/terms/day08/doc

solve : Boolean -> Text -> Nat
solve extend input =
  use Boolean not
  use List ++ +:
  use Universal neq
  use lib.base.data.List filter flatMap
  points = Point.parse input
  inLine =
    points
      |> filter (at2 >> neq ?.)
      |> lib.base.data.List.groupMapReduce at2 (p -> [at1 p]) (++)
      |> Map.values
      |> flatMap
           (antennas ->
             antennas
               |> flatMap
                    (a ->
                      lib.base.data.List.map (Tuple.pair a) antennas)
               |> filter (as -> neq (at1 as) (at2 as))
               |> flatMap (as -> [as, Tuple.swap as]))
  space = lib.base.data.List.toMap points
  getAntinodes = cases
    (p, p') ->
      _ = "diff = p' - p"
      diff = p' |> move (Point.scale -1 p)
      go antinodes p =
        use List :+
        antinode = move p diff
        if not (Map.contains antinode space) then antinodes
        else go (antinodes :+ antinode) antinode
      if not extend then go [] p' |> List.take 1 else p' +: go [] p'
  inLine
    |> flatMap getAntinodes
    |> lib.base.data.List.distinct
    |> List.size

As you can see, some List functions appear unqualified, like filter. Others are only qualified by type, like List.size. Others still are ully qualified, like lib.base.List.map . names of the fully qualified names reports a single hash.

Local display does a bit better, the qualified names aren't fully qualified, although It's still unclear to me how it decides when to qualify in the first place.

solve : Boolean -> Text -> Nat
solve extend input =
  use Boolean not
  use List ++ +: filter flatMap
  points = Point.parse input
  inLine =
    points
      |> filter (at2 >> (!==) ?.)
      |> groupMapReduce at2 (p -> [at1 p]) (++)
      |> Map.values
      |> flatMap
           (antennas ->
             antennas
               |> flatMap (a -> List.map (Tuple.pair a) antennas)
               |> filter (as -> at1 as !== at2 as)
               |> flatMap (as -> [as, Tuple.swap as]))
  space = Map.fromList points
  getAntinodes = cases
    (p, p') ->
      _ = "diff = p' - p"
      diff = p' |> move (Point.scale -1 p)
      go antinodes p =
        use List :+
        antinode = move p diff
        if not (Map.contains antinode space) then antinodes
        else go (antinodes :+ antinode) antinode
      if not extend then go [] p' |> List.take 1 else p' +: go [] p'
  inLine |> flatMap getAntinodes |> distinct |> List.size
@SystemFw SystemFw added the error-message Request for improved error message label Dec 9, 2024
@aryairani
Copy link
Contributor

aryairani commented Dec 9, 2024

We use a heuristic, which iirc, for the local version, is that it qualifies the minimum number of segments necessary to be uniquely determine the reference.

So:

x.a = "hello"
y.a = "hello"
p.b = "goodbye"
q.b = "goodbye2"
f z = z
prog = x.a ++ q.b
prog2 g = f (x.a ++ q.b ++ g)
issue5490/main> add
issue5490/main> move.term f g
issue5490/main> view prog prog2

is expected to produce:

prog = a ++ q.b -- a is not ambiguous, because x.a and y.a are the same
prog2 g = .g (a ++ q.b ++ g)

although currently prog2 prints incorrectly, with a fix in progress.

I agree the heuristic isn't great, but we haven't come up with something better.

@pchiusano
Copy link
Member

This seems like a bug with the Share rendering, it's including more segments than it should. The local and Share rendering should generally be the same.

I think I've said this before, but I'd also be in favor of just never showing more than two segments on Share. You can always hover or click through. Once you have blah.foo.bar.quaffle.List.map, your eyes glaze over anyway and it's hard to read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error-message Request for improved error message
Projects
None yet
Development

No branches or pull requests

3 participants