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

[Merged by Bors] - chore(data/finsupp/basic): make arguments explicit #14551

Closed
wants to merge 2 commits into from

Conversation

eric-wieser
Copy link
Member

This follow the pattern that arguments to an = lemma should be explicit if they're not implied by other arguments.


Open in Gitpod

@eric-wieser eric-wieser added awaiting-review The author would like community review of the PR awaiting-CI The author would like to see what CI has to say before doing more work. labels Jun 4, 2022
This follow the pattern that arguments to an `=` lemma should be explicit if they're not implied by other arguments.
@eric-wieser eric-wieser force-pushed the eric-wieser/finsupp-explicit branch from 46e754c to be0ab8d Compare June 4, 2022 10:43
@eric-wieser eric-wieser changed the title chore(data/finsupp/basic): make arguments implicit chore(data/finsupp/basic): make arguments explicit Jun 4, 2022
@fpvandoorn
Copy link
Member

I don't think this is a rule at all. There are many examples where it's more convenient to have it implicit. See e.g. closure_closure or closure_union.

Is there a single occurrence of @foo or (foo : ...) you could simplify using these changes?

@eric-wieser
Copy link
Member Author

I'd argue that the arguments should be explicit in your two examples too; it's a lot harder to target a specific rewrite if you can't fill out any of the unbound variables.

@fpvandoorn
Copy link
Member

Maybe you're right.
I think it's a lot more common that you want arguments to non-equalities to be implicit (I'm annoyed that lemmas like subset_union_(left|right) have explicit arguments), but I guess the difference is that you cannot rw with , and therefore you write term-mode proofs way more frequently than with =. This is rarely a problem with equality proofs.

bors merge

@bors
Copy link

bors bot commented Jun 4, 2022

👎 Rejected by label

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Jun 4, 2022
@fpvandoorn
Copy link
Member

good bot

bors d+

@bors
Copy link

bors bot commented Jun 4, 2022

✌️ eric-wieser can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@github-actions github-actions bot added the delegated The PR author may merge after reviewing final suggestions. label Jun 4, 2022
@eric-wieser
Copy link
Member Author

Yes, I agree that non-equalities typically have different rules.

@github-actions github-actions bot removed the awaiting-CI The author would like to see what CI has to say before doing more work. label Jun 4, 2022
@eric-wieser
Copy link
Member Author

bors merge

bors bot pushed a commit that referenced this pull request Jun 4, 2022
This follow the pattern that arguments to an `=` lemma should be explicit if they're not implied by other arguments.
@bors
Copy link

bors bot commented Jun 4, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore(data/finsupp/basic): make arguments explicit [Merged by Bors] - chore(data/finsupp/basic): make arguments explicit Jun 4, 2022
@bors bors bot closed this Jun 4, 2022
@bors bors bot deleted the eric-wieser/finsupp-explicit branch June 4, 2022 17:33
tomaz1502 pushed a commit that referenced this pull request Jun 5, 2022
This follow the pattern that arguments to an `=` lemma should be explicit if they're not implied by other arguments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions. ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants