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

Documentation of rewrite occurrences is incorrect and incomplete #15591

Open
JasonGross opened this issue Feb 2, 2022 · 6 comments
Open

Documentation of rewrite occurrences is incorrect and incomplete #15591

JasonGross opened this issue Feb 2, 2022 · 6 comments
Labels
kind: bug An error, flaw, fault or unintended behaviour. kind: documentation Additions or improvement to documentation. part: rewriting tactics The rewrite, autorewrite, rewrite_strat, and setoid_rewrite tactics.

Comments

@JasonGross
Copy link
Member

JasonGross commented Feb 2, 2022

Description of the problem

https://coq.inria.fr/refman/proofs/writing-proofs/equality.html#coq:tacn.rewrite says of occurrences:

If occurrences specifies multiple occurrences, the tactic succeeds if any of them can be rewritten. If not specified, only the first occurrence in the conclusion is replaced.

This neglects the existence of occurrences in the hypotheses:

Lemma foo (f : nat -> nat) (x y : nat) (H : forall a, a = x -> f a = x) (H' : f y = f y) : (f x = f y).
Proof.
  rewrite H in H' |- * by reflexivity. (* fails *)

Perhaps "any" should be "all"?

cc @jfehrle

Coq Version

8.16, master

@JasonGross JasonGross added kind: documentation Additions or improvement to documentation. kind: bug An error, flaw, fault or unintended behaviour. part: rewriting tactics The rewrite, autorewrite, rewrite_strat, and setoid_rewrite tactics. labels Feb 2, 2022
@jfehrle
Copy link
Member

jfehrle commented Feb 4, 2022

I'm not a subject matter expert on this, so I don't have an opinion. Is it a code issue or a doc issue, i.e., should rewrite behave this way for your example? (BTW, there's an extraneous comma in the example.)

@JasonGross
Copy link
Member Author

This is a doc issue. The example I gave here has the correct behavior. There's a code issue at #15592, which is about subtleties in interactions between occurrences and multiple expressions.

Where's the extra comma?

@jfehrle
Copy link
Member

jfehrle commented Feb 4, 2022

This is a doc issue. The example I gave here has the correct behavior.

@SkySkimmer do you agree?

@JasonGross Then what should the doc say?

The extra comma is from @SkySkimmer's edit:

image

@SkySkimmer
Copy link
Contributor

The extra comma is from @SkySkimmer's edit:

fixed

@JasonGross
Copy link
Member Author

@JasonGross Then what should the doc say?

I think if you replace "any" with "all", then it at least becomes more correct. It's still under-documented, and the behavior is subtle and intricate enough (cf #15592) that it's not clear to me what to say to fully explain the behavior.

@jfehrle
Copy link
Member

jfehrle commented Feb 7, 2022

Why don't you submit the one-word change you suggest as a PR and cut out the middleman (i.e. me)? That's less effort for everyone.

rewrite is used so frequently that the doc should have a precise and complete description, including for occurrences. If there's not a simple, relatively intuitive description (a couple sentences) and there aren't excellent reasons for it to be complicated, then we should look at changing the behavior to make it simple. We'd need to consider the compatibility impact. It might need a compatibility flag (if feasible to support) or it may affect too much user code to be a viable change.

Shouldn't it be relatively easy to figure out the behavior by examining the source code for the tactic?

@Alizter Alizter added this to Fixing in User documentation May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug An error, flaw, fault or unintended behaviour. kind: documentation Additions or improvement to documentation. part: rewriting tactics The rewrite, autorewrite, rewrite_strat, and setoid_rewrite tactics.
Projects
Status: Fixing
Development

No branches or pull requests

3 participants