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] - refactor(tactic/lift): remove attribute/simp-set and swap side goal #16565
Conversation
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.
The change seems reasonable to me. Sticking things in out_params
seems like a fine design for meta code, where you're just using the typeclass instances as a database.
@@ -335,7 +335,7 @@ def const_vadd_hom : multiplicative G →* equiv.perm P := | |||
|
|||
variable {P} | |||
|
|||
open function | |||
open _root_.function |
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.
Is the _root_
necessary, or is it vestigial?
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.
It is necessary, for reasons I don't understand.
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.
Oh, it's because there's an equiv.function
namespace. I didn't know that _root_
worked in open
commands.
I suppose this is fine, but I checked that you can put open function
near the beginning of the module. If you don't think that needs to be done, then the PR looks good to me.
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 would prefer to keep it like this.
Thanks! bors r+ |
…16565) This PR accomplishes two things: * It simplifies the design of the `lift` tactic (while keeping roughly the same user interface, see below). Specifically, it does not need an attribute set to simplify expressions coming from `can_lift` instances. - This will make the tactic easier to port to Lean 4. - It accomplishes this by moving two fields of `can_lift` into `out_param`s. So writing the instances is slightly different from before. * If the `using h` clause is left out, then `lift` produces a subgoal. This subgoal used to come after the main goal. But I think it is natural to make it the first goal. So that you can write ``` lift n to nat with k, { linarith }, ``` instead of ``` lift n to nat using by { linarith } with k, ```
Pull request successfully merged into master. Build succeeded: |
This PR accomplishes two things:
lift
tactic (while keeping roughly the same user interface, see below). Specifically, it does not need an attribute set to simplify expressions coming fromcan_lift
instances.can_lift
intoout_param
s. So writing the instances is slightly different from before.using h
clause is left out, thenlift
produces a subgoal. This subgoal used to come after the main goal. But I think it is natural to make it the first goal. So that you can writeinstead of