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

feat(library/init/meta/simp_tactic) simp with reversed lemmas #100

Merged
merged 16 commits into from Feb 19, 2020

Conversation

Vierkantor
Copy link
Collaborator

Pull Request Description

Allow the simp tactic to apply reverse direction, analogously to rw: if h : a = b, then simp [<-h] will replace all instances of b in the goal with a.

This implementation is inspired by rewrite: we store a flag for each expr lemma passed to simp, indicating whether to reverse it. When all parameters to the lemma have been instantiated (so that the lemma has the form h _ ... _ : R _ _, mk_symm is applied to it to get the reverse relation.

library/init/meta/simp_tactic.lean Show resolved Hide resolved
library/init/meta/simp_tactic.lean Outdated Show resolved Hide resolved
library/init/meta/simp_tactic.lean Outdated Show resolved Hide resolved
src/library/tactic/simp_lemmas.h Show resolved Hide resolved
@Vierkantor
Copy link
Collaborator Author

I'm currently compiling the full mathlib against the changes, but from testing mathlib/src/tactics, it seems that only squeeze_simp needs some fixes. Here is the PR to mathlib to fix this: leanprover-community/mathlib#1923

Vierkantor and others added 14 commits January 30, 2020 13:09
Co-Authored-By: Gabriel Ebner <gebner@gebner.org>
…mpatible

The problem is that some tactics in `mathlib` extending `simp` do not handle
the `symm` argument, so by introducing the new behavior under a new name and
erroring out when `symm` is found in the old behavior, calls (but not
pattern-matching) shouldn't break too much.
In `mathlib`, a similar function was used by `squeeze_simp`, but changes in the
definition for `simp_arg_type` will break this.  This solution adds a polyfill
instance in `mathlib` with low priority and the "real" instance with high
priority in the new version of core Lean.
If there is a `lemma @[simp] foo` and we call `simp [<-foo]`, this would give a
timeout because it will start rewriting back and forth from LHS to RHS. To
avoid this, we delete all `foo`s coming from annotations when collecting lemmas
for `simp [<-foo]`.
@Vierkantor
Copy link
Collaborator Author

After updating to the master branch, mathlib builds with no errors!

I've added some code to prevent conflicts between forwards and backwards rewriting rules (preventing an infinite loop of rewriting back-and-forth): if there is a @[simp] lemma foo and simp [<-foo] is called, the original lemma foo is deleted from the set of simp lemmas so that only foo.symm remains.

But I just realized that this doesn't solve all conflicts: if for whatever reason there is a @[simp] lemma bar with the same type as foo, simp [<-foo] would be equivalent to simp [<-foo, bar], and thus start looping again. I guess the simplest way to prevent all these issues is to say [<-foo] is only allowed as an argument to simp only so conflicts should be easily apparent. Or is this just something that the user is expected to be careful with?

@gebner
Copy link
Member

gebner commented Feb 11, 2020

Or is this just something that the user is expected to be careful with?

Yes, I think this is up to the user. You can already very easily make simp loop by calling simp *, so this issue is not new.

@gebner
Copy link
Member

gebner commented Feb 11, 2020

Or is this just something that the user is expected to be careful with?

Yes, I think this is up to the user. You can already very easily make simp loop by calling simp *, so this issue is not new.

LGTM.

@gebner gebner merged commit 6d30168 into leanprover-community:master Feb 19, 2020
@cipher1024
Copy link
Contributor

What's going on with the build?

@bryangingechen
Copy link
Collaborator

@cipher1024 From what I can tell, something went wrong here causing ccache not to be installed. Here's the relevant part of .travis.yml. Not sure if something changed with homebrew or with travis...

@gebner
Copy link
Member

gebner commented Feb 19, 2020

This is a bit surprising, the CI worked on the PR.

@gebner
Copy link
Member

gebner commented Feb 19, 2020

See #116.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants