Make rapply handle all numbers of underscores#10760
Conversation
|
I shall investigate the mathclasses failure (it is a genuine bug with this PR, though I'm not sure what the bug is yet). Can someone from @coq/doc-maintainers tell me the suggested fix for ? Is |
|
|
We don't make any difference between built-in and in standard library, but all the tactics should be documented within the reference manual, and that's the only way we can reference them later on. For instance |
Zimmi48
left a comment
There was a problem hiding this comment.
Changelog entry looks good. Would be really great if you could take advantage of this PR to add documentation of rapply to the Tactics chapter.
Documentation for |
|
The incompatibility with math-classes is a real one. The issue is that they frequently rely on the fact that
Thoughts? cc @spitters |
This backwards compatible change makes math-classes work with rocq-prover/rocq#10760 by replacing all instances of `rapply` which were relying on typeclass resolution happening *before* `refine` (instead of simultaneously with it) to instead invoke `Tactics.rapply` (which is the same tactic in Coq <= 8.10, and which will be the tactic rather than the `uconstr`-taking tactic notation in Coq >= 8.11). See also rocq-prover/rocq#10760 (comment)
|
Overlay added for math-classes (rocq-community/math-classes#78). For reference, I needed to change 9 of the 34 uses of |
|
Your patch and the motivation seem not the same. I was expecting one more line with more _ but your change is more ambitious and indeed you get a good deal of breakage. Would it make sense to try the more conservative approach? |
|
I didn't know what |
|
@gares if I remove the @Zimmi48 indeed, if someone wants to redefine |
ba77093 to
2d53e80
Compare
|
I've removed the tactic notation, the overlay, and updated the doc about the more complicated spec of |
2d53e80 to
f993d15
Compare
|
Build failure is unrelated to this PR, as it also occurs in master. |
f993d15 to
5635b3c
Compare
|
Lots of commits, maybe could use a squash. |
5635b3c to
5a28203
Compare
|
Squashed to three commits (I want to leave the uconstr version of rapply in version history, in case anyone wants to come back to it). |
5a28203 to
7b1dc94
Compare
|
It's not my territory and I'm not very competent on that (last time I've tried to use |
|
Should this get a bench? |
Also add a tactic notation so that it takes in uconstrs by default. Also add some basic tests for `rapply`. Also document rapply in the manual
This increases backwards compatibility. If desired, we can add a tactic notation to simplify the spec of `rapply` in the future if we want.
7b1dc94 to
8d9afb9
Compare
|
I've rebased, and started a bench at https://ci.inria.fr/coq/view/opam/job/benchmark-part-of-the-branch/810/consoleText |
|
Partial bench: Since this one was interrupted, I've spun up a new one at https://ci.inria.fr/coq/view/opam/job/benchmark-part-of-the-branch/813/consoleText . But so far it looks to be mostly noise. |
|
Another partial bench result: Trying once more for everything at https://ci.inria.fr/coq/view/opam/job/benchmark-part-of-the-branch/816/consoleText |
|
Everything except for coq-rewriter-perf: I think this is within noise, and hence ready to be merged. |
|
Let's merge, this has waited for too long already. |
Ack-by: Zimmi48 Reviewed-by: ppedrot
Kind: enhancement.
Closes #10004
Also add a tactic notation so that it takes in uconstrs by default.
Also add some basic tests for
rapply.I made this PR because I hit a case where I needed 16 underscores (the existing
rapplyonly gives me 15).