-
Notifications
You must be signed in to change notification settings - Fork 260
[ refactor ] use variables more systematically in Data.List.Fresh{.*}
#2916
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like these changes! The only comment I have is that according to the style guide, P should not be a variable as it is not involved in the type of the data definition.
EDIT: Ah I see you already commented above. So R is covered under the style guide already. I think the current style-guide rule is a good balance, and I don't see how one could extend it in a principled manner?
@MatthewDaggitt , you wrote
Thanks! UPDATED: TL;DR: you were right, and I was wrong. The additional commits for the (IGNORE FROM HERE) I (largely) agree, but in my more 'maximalist' style, I am reaching the point where I would like the
That is to say, in this example, once we have Now, this would be a significant widening of the style-guide recommendations, for sure, so how to keep things under reasonable control so that
The alternative (as we do now, but sometimes it gets very creaky, at least, that was how I felt doing this piece of work) is to be (more) disciplined about using module _ {P : Pred A _} wherescoping mechanisms, but the creakiness comes, as with this PR when we pass to I'm sure there are good reasons to reject the above ideas, but... testing the envelope! UPDATED: I think the above can (mostly) stand, but I did at least re-read my own code contributed here, and agree with you. I was being an idiot! |
|
I think in the interests of review overhead, I won't contribute the corresponding refactoring of |
| -- Convenient notation for freshness making A and R implicit parameters | ||
| infix 5 _#_ | ||
| _#_ : {R : Rel A r} (a : A) (as : List# A R) → Set r | ||
| _#_ : REL A (List# A R) _ | ||
| _#_ = fresh _ _ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole intention behind this PR is to better support generalisation over R when using List#, and specifically the ability to generalise over arguments xs : List# A R correctly.
Unfortunately, this sometime means that some statements involving x # xs fail to generalise correctly wrt the implied/implicit argument R relative to other generalisation sites in the type. My current solution is to nudge the type checker by sprinkling a few {R = R} implicit bindings where needed, but this seems slightly less satisfactory, when the real culprit seems to be the relation _#_
I see two possible solutions:
- delegate to using the explicit
freshoperation where needed for disambiguation; but this changes the 'visible'/'legible' API when stating lemmas - introduce a small additional piece of notation, eg
x #[ R ] xswhereRneeds to be made visible (and for some reason I couldn't seem to do this assyntax...?)
Thoughts? Eg. as
-- Convenient notation for freshness making A (and R) implicit
infix 5 _#[_]_ _#_
_#[_]_ : A → (R : Rel A r) → Pred (List# A R) _
x #[ R ] xs = fresh _ R x xs
_#_ : REL A (List# A R) _
x # xs = x #[ _ ] xs| take : {R : Rel A r} → ℕ → List# A R → List# A R | ||
| take-# : {R : Rel A r} → ∀ n a (as : List# A R) → a # as → a # take n as | ||
| take : ℕ → List# A R → List# A R | ||
| take-# : ∀ n y (xs : List# A R) → y # xs → y # take n xs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eg if we were to introduce _#[_]_ then this could typecheck as:
| take-# : ∀ n y (xs : List# A R) → y # xs → y # take n xs | |
| take-# : ∀ n y xs → y #[ R ] xs → y # take n xs |
| takeWhile : {R : Rel A r} → List# A R → List# A R | ||
| takeWhile-# : ∀ {R : Rel A r} a (as : List# A R) → a # as → a # takeWhile as | ||
| takeWhile : List# A R → List# A R | ||
| takeWhile-# : ∀ y (xs : List# A R) → y # xs → y # takeWhile xs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly
| takeWhile-# : ∀ y (xs : List# A R) → y # xs → y # takeWhile xs | |
| takeWhile-# : ∀ y xs → y #[ R ] xs → y # takeWhile xs |
| filter : {R : Rel A r} → List# A R → List# A R | ||
| filter-# : ∀ {R : Rel A r} a (as : List# A R) → a # as → a # filter as | ||
| filter : List# A R → List# A R | ||
| filter-# : ∀ y (xs : List# A R) → y # xs → y # filter xs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| filter-# : ∀ y (xs : List# A R) → y # xs → y # filter xs | |
| filter-# : ∀ y xs → y #[ R ] xs → y # filter xs |
etc.
| toList : {R : Rel A r} → List# A R → ∃ (AllPairs R) | ||
| toAll : ∀ {R : Rel A r} {a} as → fresh A R a as → All (R a) (proj₁ (toList as)) | ||
| toList : List# A R → ∃ (AllPairs R) | ||
| toAll : (xs : List# A R) → x # xs → All (R x) (proj₁ (toList xs)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| toAll : (xs : List# A R) → x # xs → All (R x) (proj₁ (toList xs)) | |
| toAll : ∀ xs → x #[ R ] xs → All (R x) (proj₁ (toList xs)) |
| -- then we have a list of distinct values. | ||
|
|
||
| module _ {a} (A : Set a) (R : Rel A r) where | ||
| module _ (A : Set a) (R : Rel A r) where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I keep scratching my head as to whether A could be bound implicitly here, or indeed should.
That would obviously be a breaking change to the API, so out-of-scope for this PR, but still...
| there : ∀ {x xs pr} → Any xs → Any (cons x xs pr) | ||
|
|
||
| module _ {R : Rel A r} {P : Pred A p} {x} {xs : List# A R} {pr} where | ||
| module _ {pr : fresh A R x xs} where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the one glaring wart in the refactoring, but I couldn't find a clean way out of the bottle, other than to introduce the abbreviation pr : x #[ R ] xs. Suggestions welcome!
UPDATED: this seems to be a problem with the relative order in which P gets generalised... But simple-minded permutation of the varaiable block doesn't seem to fix it. Oh well.
@jkopanski 's recent #2914 nudged me to think about refactoring the associated modules
variables (UPDATED: it seems the only reasonable way to do this for the whole library is ... little-by-little, at least wrt only 'small' groups of cognate modules?)RelandPredarguments... (elsewhere discussed as astyle-guide/library-designissue Policy on generalised variables #697 [ refactor ] Experimenting with variables inData.List#701 etc.)DRAFT, for discussion at this stage.Now open for review (including a self-review... ;-))No
CHANGELOG. If not exactly 'cosmetic'...