unisonweb / unison

A friendly programming language from the future
https://unison-lang.org
Other
5.81k stars 271 forks source link

dependents doesn't catch type dependencies for literal values #5346

Closed ceedubs closed 2 months ago

ceedubs commented 2 months ago

Describe and demonstrate the bug

If a term has Doc in the type signature (such as its return type being Doc) then it is recognized as a dependent of Doc. But if the term has an internal Doc literal, then it doesn't get flagged as being dependent on Doc.

Edit: This seems to be true for literals of any builtin type (Nat, Bytes, Text, etc); not just Doc.

test/main> view test

  test : '()
  test = do ignore {{ This function doesn't depend on Doc? }}

InputPattern: 13.8 ms (cpu), 15.5 ms (system)
test/main> view test2

  test2 : Doc
  test2 = {{ This function returns a Doc }}

InputPattern: 3.05 ms (cpu), 4.81 ms (system)
test/main> dependents lib.base.Doc

  Dependents of: type Doc

    Terms:

    1. test2

  Tip: Try `view 1` to see the source of any numbered item in the above list.

Additional context

This came up when I was trying to identify anywhere that doc literals were being used for inline comments that might cause unnecessary runtime overhead when the doc was really just meant for the reader of the code and not something that we care about evaluating at runtime.

ceedubs commented 2 months ago

Oh this goes beyond Doc. I see the same thing for literals of Nat, Bytes, Text, and presumably any other builtin.

pchiusano commented 2 months ago

Aside: Doc literals aren't AST elements like Nat is, they are just syntax sugar for regular function calls. The treatment of Nat literals and whether they count as a dependency on Nat is separate.

dependencies / dependents will only show types that are specifically mentioned in a type signature of the function, or types with constructors that you explicitly call or pattern match in the function. It does not like look up the type signature of every function you call internally and record that as a type dependency. Should it?Transitive dependents / dependencies would reveal this.

pchiusano commented 2 months ago

dependents docParagraph or dependents docWord should do it since doc literals desugar into calls to those functions (among others).

ceedubs commented 2 months ago

Based on Paul's comment I'm going to close this out. It's not clear that the current behavior is wrong. If we determine it to be an issue we can always reopen.