-
Notifications
You must be signed in to change notification settings - Fork 199
Adapt to Rocq#21180 #2315
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?
Adapt to Rocq#21180 #2315
Conversation
jdchristensen
left a comment
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 guess an alternative is a custom `justsplit' tactic that does nothing on equality goals and does split on all other goals...
theories/Algebra/Groups/FreeGroup.v
Outdated
| snapply (Build_Group freegroup_type). | ||
| - exact _. | ||
| - exact _. | ||
| - exact _. | ||
| - split. | ||
| + repeat split; exact _. | ||
| + exact _. | ||
| + exact _. |
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'm curious how reflexivity could possibly solve any of the goals here. That doesn't look possible to me. And it's a bit annoying to be unable to use `repeat split'.
Do either of the following work?
snapply (Build_Group freegroup_type).
4: repeat split.
all: exact _.
or with the second line replaced with
4: split. 4: split. 4: split.
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.
One of the goals is the fact that inv x is the left-inverse of x. Since the unit and the operation are not yet instantiated, there is a solution that makes op the constant operation returning the unit.
Oh, good catch, 4: repeat split causes the issue, but 4: split. 4: repeat split does work, thank you.
| snapply (Build_Group amal_type). | ||
| - exact _. | ||
| - exact _. | ||
| - exact _. | ||
| - split. | ||
| + repeat split; exact _. | ||
| + exact _. | ||
| + exact _. |
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'd also prefer something more concise here.
|
I can do that, if you prefer. I just need to know where to put the tactic. |
jdchristensen
left a comment
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 think this looks fine now, without needing a custom tactic. I think we should only merge it if the Coq change is definitely going to be merged. Do we know that yet or should I wait to find out?
|
There is no hurry, I still need to make a few overlays (or fix bugs). |
|
@Tragicus , is it possible to tell Coq to run a tactic like Also, based on your diagnosis, I suspect that this single line would work for the first hunk: (I'm not sure why |
(I think the reason is that other arguments depend on this one, and it doesn't know how to solve them. So while it finds the |
|
I do not think so, but your fix also works. Which one do you prefer? |
jdchristensen
left a comment
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 the latest version the most. It's more robust, as once you've specified the operation, the remaining things are properties.
There are a few
repeat splitin definitions of groups, which causessplitto try and solve equalities using reflexivity. This used to fail, but with rocq-prover/rocq#21180 the unification algorithm becomes powerful enough to find a solution, albeit not the required one.