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] - fix(tactic/core): Make the classical tactic behave like open_locale classical #14122

Closed
wants to merge 22 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented May 13, 2022

This renames the existing classical tactic to classical!, and adds a new classical tactic that is equivalent to open_locale classical.

Comparing the effects of these:

import tactic.interactive
import tactic.localized

-- this uses the noncomputable instance
noncomputable def foo : decidable_eq ℕ :=
λ m n, begin classical!, apply_instance, end

def bar : decidable_eq ℕ :=
λ m n, begin classical, apply_instance, end

section
open_locale classical

def baz : decidable_eq ℕ :=
λ m n, by apply_instance

end

example : baz = bar := rfl

In a few places classical was actually just being used as a resetI. Only a very small number of uses of classical actually needed the more aggressive classical! (there are roughtly 500 uses in total); while a number of congrs can be eliminated with this change.


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 May 13, 2022
@fpvandoorn
Copy link
Member

Note: if this works, we probably want to mimic open_locale classical even better, which does a little more

@eric-wieser
Copy link
Member Author

Alternatively, we might want to just remove those lines from the classical locale; it's not immediately clear to me what purpose they serve.

@eric-wieser
Copy link
Member Author

eric-wieser commented May 13, 2022

Oh, actually decidable_eq_of_decidable_le has been de-instanced upstream anyway

@fpvandoorn
Copy link
Member

Presumably it is to deal with performance issues causes by those instances. But the de-instantiated instances should of course be removed.

@gebner gebner added the modifies-tactic-syntax This PR adds a new interactive tactic or modifies the syntax of an existing tactic. label May 14, 2022
@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 May 15, 2022
@fpvandoorn
Copy link
Member

This looks great!

bors merge

@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 May 16, 2022
bors bot pushed a commit that referenced this pull request May 16, 2022
…e classical` (#14122)

This renames the existing `classical` tactic to `classical!`, and adds a new `classical` tactic that is equivalent to `open_locale classical`.

Comparing the effects of these:
```lean
import tactic.interactive
import tactic.localized

-- this uses the noncomputable instance
noncomputable def foo : decidable_eq ℕ :=
λ m n, begin classical!, apply_instance, end

def bar : decidable_eq ℕ :=
λ m n, begin classical, apply_instance, end

section
open_locale classical

def baz : decidable_eq ℕ :=
λ m n, by apply_instance

end

example : baz = bar := rfl
```

In a few places `classical` was actually just being used as a `resetI`. Only a very small number of uses of `classical` actually needed the more aggressive `classical!` (there are roughtly 500 uses in total); while a number of `congr`s can be eliminated with this change.
@bors
Copy link

bors bot commented May 16, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title fix(tactic/core): Make the classical tactic behave like open_locale classical [Merged by Bors] - fix(tactic/core): Make the classical tactic behave like open_locale classical May 16, 2022
@bors bors bot closed this May 16, 2022
@bors bors bot deleted the eric-wieser/fix-classical branch May 16, 2022 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modifies-tactic-syntax This PR adds a new interactive tactic or modifies the syntax of an existing tactic. 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

3 participants