Skip to content

implement dependents <x>, dependencies <x>, and debug.file #1401

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

Merged
merged 6 commits into from
Apr 8, 2020

Conversation

aryairani
Copy link
Contributor

@aryairani aryairani commented Apr 4, 2020

Overview

This PR adds three commands.

debug.file

I can use debug.file to see the hashes of the last typechecked file.

Given this .u file:

type outside.A = A Nat outside.B
type outside.B = B Int
outside.c = 3
outside.d = c < (p + 1)

type inside.M = M outside.A
inside.p = c
inside.q x = x + p * p
inside.r = d
.> 

  I found and typechecked these definitions in u.u. If you do an `add` or `update`, here's how your
  codebase would change:
  
    ⍟ These new definitions are ok to `add`:
    
      type inside.M
      type outside.A
      type outside.B
      inside.p  : Nat
      inside.q  : Nat -> Nat
      inside.r  : Boolean
      outside.c : Nat
      outside.d : Boolean
   
  Now evaluating any watch expressions (lines starting with `>`)... Ctrl+C cancels.

.> debug.file

  type inside.M#4idrjau939
  type outside.A#0n4pbd0q9u
  type outside.B#muulibntaq
  inside.p#fiupm7pl7o
  inside.q#l5pndeifuh
  inside.r#im2kiu2hmn
  outside.c#msp7bv40rv
  outside.d#6cdi7g1oi2

This will help me make progress in some situations when UCM is being deficient or broken.

dependents / dependencies

But wait, there's more. I can check the dependencies and dependents of a definition:

.> add

  ⍟ I've added these definitions:
  
    type inside.M
    type outside.A
    type outside.B
    inside.p  : Nat
    inside.q  : Nat -> Nat
    inside.r  : Boolean
    outside.c : Nat
    outside.d : Boolean

.> dependents q

  #l5pndeifuh doesn't have any dependents.

.> dependencies q

  Dependencies of #l5pndeifuh:
  
    Hash          Name
    ##Nat.*       base.Nat.*
    ##Nat.+       base.Nat.+
    #fiupm7pl7o   inside.p

.> dependencies B

  Dependencies of #muulibntaq:
  
    Hash    Name
    ##Int   base.Int


  Dependencies of #muulibntaq#0:
  
    Hash          Name
    ##Int         base.Int
    #muulibntaq   outside.B

.> dependencies d

  Dependencies of #6cdi7g1oi2:
  
    Hash            Name
    ##Nat           base.Nat
    ##Nat.+         base.Nat.+
    ##Universal.<   base.Universal.<
    #fiupm7pl7o     inside.p
    #msp7bv40rv     outside.c

.> dependents d

  Dependents of #6cdi7g1oi2:
  
    Hash          Name
    #im2kiu2hmn   inside.r

.> 

I guess this will close #451, which asks for dependencies and/or transitive dependencies.

Implementation notes

We don't have an index for dependents of constructors, but iirc if you ask for that, it will show you dependents of the type that provided the constructor.

Working on this reminded me that LabeledDependency exists, and we could probably use it in more places to clean up implementations. LabeledReference would probably be a better name now.

-- constructor name is private, the module just exposes `fold`
newtype LabeledDependency = X (Either Reference Referent) deriving (Eq, Ord, Show)

I added the following function in HandleInput.hs, which looks similar to many others in there. It might be the best one ever, or it might be a duplicate, we'll have to take a look when we clean up that file.

resolveHQToLabeledDependencies 
  :: Functor m => HQ.HashQualified -> Action' m v (Set LabeledDependency)`

Testing

I caught some bugs through manual testing (e.g. fixed #1399), but it would be good to add some tests in not too long, to prevent regression. I haven't thought too much about what they'd look like, though.

Loose ends

  • It might be nice if a query name was in the output too, but I am not too worried about it.
  • It might be nice to have a flag to include transitive dependencies, but I am not too worried about it.

Comment on lines +986 to +987
-- this definition is identical to the previous one, apart from the word
-- "Dependencies", but undecided about whether or how to refactor
Copy link
Contributor Author

@aryairani aryairani Apr 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

¯\_(ツ)_/¯

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: I'd do column2sep (you'll have to write it, can pattern after column3sep, just add sep before the right column) to add an extra space between the columns, and I'd also add a header:

  P.column2Sep "  " $ [ (P.hiBlack "Hash", P.hiBlack "Name") ] ++ ...

Or maybe better yet, add a column2Header : Pretty -> Pretty -> [(Pretty, Pretty)] -> Pretty that encapsulates this pattern of adding a header.

It's also okay as is if you are out of steam, but I had a hard time parsing the current output and think above suggestion might help.

@aryairani aryairani marked this pull request as ready for review April 4, 2020 03:19
@aryairani aryairani requested review from pchiusano, mitchellwrosen and runarorama and removed request for mitchellwrosen April 4, 2020 03:19
Copy link
Member

@pchiusano pchiusano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Can you either do the following as part of this PR or create tickets to track the following (and if you disagree on any of these being a good idea LMK) -

  1. Make dependents and dependencies commands produce numbered output
  2. Add a transcript test (once the commands produce numbered output, create simple transcript to exercise)
  3. Make these commands produce output just like find - show type signatures and whatnot

No need to address any/all of those in this PR I don't think, unless you are feeling inspired.

I left one other minor formatting suggestion on the output, also optional I think.

Comment on lines +986 to +987
-- this definition is identical to the previous one, apart from the word
-- "Dependencies", but undecided about whether or how to refactor
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: I'd do column2sep (you'll have to write it, can pattern after column3sep, just add sep before the right column) to add an extra space between the columns, and I'd also add a header:

  P.column2Sep "  " $ [ (P.hiBlack "Hash", P.hiBlack "Name") ] ++ ...

Or maybe better yet, add a column2Header : Pretty -> Pretty -> [(Pretty, Pretty)] -> Pretty that encapsulates this pattern of adding a header.

It's also okay as is if you are out of steam, but I had a hard time parsing the current output and think above suggestion might help.

@aryairani
Copy link
Contributor Author

@pchiusano Sounds good. I'll add the column headings, then maybe do {1,2}, and open a ticket to track {3}.

@aryairani aryairani added the ready-to-merge Apply this to a PR and it will get merged automatically once CI passes and 1 reviewer has approved label Apr 8, 2020
@mergify mergify bot merged commit e7fd115 into master Apr 8, 2020
@mergify mergify bot deleted the topic/dependents-dependencies-debug.file branch April 8, 2020 00:27
@mergify mergify bot removed the ready-to-merge Apply this to a PR and it will get merged automatically once CI passes and 1 reviewer has approved label Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

alias.term doesn't work for constructor hashes add CLI command for listing dependents and/or transitive dependents of a definition
2 participants