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

[refactor] combine checkInputs/Outputs and use for(M) #42

Merged
merged 11 commits into from
Nov 1, 2024

Conversation

acl-cqc
Copy link
Collaborator

@acl-cqc acl-cqc commented Oct 21, 2024

There were two goals here:

  • combine the common parts of checkInputs/Outputs, that is, they are the same apart from flipping of Src/Tgt (with Bool flag to checkWire) and the error message
  • Get a list of Src+Tgt pairs, and forM over them - this should make it nice and easy to use Fork in Multi-threaded type inference #41

Felt I jumped through a few hoops to preserve behaviour, but maybe that's worth it, specifically I liked doing the do-the-wires-match-types before the "is there anything leftover" check.

Individual commits should all compile if you want to look at some intermediates (these may be plausible endpoints)

brat/Brat/Checker.hs Outdated Show resolved Hide resolved
extractSuffixes [] bs = ([], Left $ TypeErr $ errMsg ++ showRow bs ++ " for " ++ show tm)
extractSuffixes (a:as) (b:bs) = first ((a:|as,b:|bs):) $ extractSuffixes as bs

checkInputs :: forall m d . (CheckConstraints m KVerb, ?my :: Modey m)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Of course if we got rid of the extensive existing type signatures for checkInputs and checkOutputs then the line count here would look much better ;-). (Each of checkInputs and checkOutputs is used twice though so I don't think inlining is a good idea)

addRowContext exp act = \case
(Err fc (TypeMismatch tm _ _)) -> Err fc $ TypeMismatch tm exp act
e -> e
extractSuffixes as [] = ([], Right as)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Type signature here is

 [(NamedPort exp, BinderType m)]
 -> [(NamedPort act, BinderType m)]
 -> ([(NonEmpty (NamedPort exp, BinderType m), NonEmpty (NamedPort act, BinderType m))]
    , Either Error [(NamedPort exp, BinderType m)]
    )

if you really want it (!)...perhaps with a type TypedPort e m = (NamedPort e, BinderType m) ??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This might be better called zipSuffixes or zipWithSuffixes, though

@acl-cqc acl-cqc requested a review from croyzor October 21, 2024 13:07
@acl-cqc acl-cqc merged commit 8b6c4c5 into main Nov 1, 2024
1 check passed
@acl-cqc acl-cqc deleted the acl/refactor-checkInputsOutputs branch November 1, 2024 11:25
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.

2 participants