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(tactic): add various tactics about local definitions #1953

Merged
merged 7 commits into from Feb 21, 2020

Conversation

fpvandoorn
Copy link
Member

TO CONTRIBUTORS:

Make sure you have:

  • reviewed and applied the coding style: coding, naming
  • reviewed and applied the documentation requirements
  • for tactics:
  • make sure definitions and lemmas are put in the right files
  • make sure definitions and lemmas are not redundant

If this PR is related to a discussion on Zulip, please include a link in the discussion.

For reviewers: code review check list

src/data/option/defs.lean Outdated Show resolved Hide resolved
@fpvandoorn
Copy link
Member Author

fpvandoorn commented Feb 5, 2020

@cipher1024: What does the following tactic do?

/-- tactic-facing function, similar to `interactive.tactic.generalize` with the
exception that meta variables -/
meta def generalize' (h : name) (v : expr) (x : name) : tactic (expr × expr) :=

The docstring is a sentence fragment. Can I rename it to monotonicity.generalize'?

Also, many of the declarations in that file are not interactive tactics at all...

src/tactic/core.lean Outdated Show resolved Hide resolved
src/tactic/core.lean Outdated Show resolved Hide resolved
src/tactic/interactive.lean Show resolved Hide resolved
src/tactic/core.lean Outdated Show resolved Hide resolved

/-- `local_def_value e` returns the value of the expression `e`, assuming that `e` has been defined
locally using a `let` expression. Otherwise it fails. -/
meta def local_def_value (e : expr) : tactic expr :=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this and the following be better implemented in C++? IIRC local definitions are stored in the type context, right? @EdAyers exposed some of the type context to the tactic monad in 3.5c. Rather than this kind of hackish way to get the local def value, maybe there should be a function to query the type context directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would almost certainly be better to implement this in C++.
I don't know how to do that, though.

Shall merge this PR and change it until we've exposed it from the C++ side?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is already implemented:

tactic.unsafe.type_context.run $ do
lctx <- get_local_context,
some ldecl <- pure (lctx.get_local_decl e.local_uniq_name),
some let_val <- pure ldecl.value,
pure let_val

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Do you know if it possible to run tactic.pp inside the type_context monad (for error message handling)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find anything, and I don't think there is a way to run tactics from the type context monad.

@robertylewis robertylewis added the awaiting-author A reviewer has asked the author a question or requested changes label Feb 5, 2020
src/tactic/core.lean Outdated Show resolved Hide resolved

/-- `local_def_value e` returns the value of the expression `e`, assuming that `e` has been defined
locally using a `let` expression. Otherwise it fails. -/
meta def local_def_value (e : expr) : tactic expr :=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is already implemented:

tactic.unsafe.type_context.run $ do
lctx <- get_local_context,
some ldecl <- pure (lctx.get_local_decl e.local_uniq_name),
some let_val <- pure ldecl.value,
pure let_val

src/tactic/core.lean Show resolved Hide resolved
src/tactic/core.lean Outdated Show resolved Hide resolved
src/tactic/core.lean Outdated Show resolved Hide resolved
src/tactic/monotonicity/interactive.lean Outdated Show resolved Hide resolved
src/tactic/interactive.lean Outdated Show resolved Hide resolved
@gebner
Copy link
Member

gebner commented Feb 11, 2020

I don't think we need to change local_def_value and is_local_def to use the type_context monad. The current implementation seems robust enough. But please try it out if you think it's fun.

@fpvandoorn fpvandoorn added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Feb 20, 2020
@fpvandoorn
Copy link
Member Author

I updated local_def_value to use the type_context. is_local_def is a one-liner, and I kept it the same. I also added tests to check that it works.


/-- `local_def_value e` returns the value of the expression `e`, assuming that `e` has been defined
locally using a `let` expression. Otherwise it fails. -/
meta def local_def_value (e : expr) : tactic expr :=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find anything, and I don't think there is a way to run tactics from the type context monad.

`generalize'` is similar to `generalize`. The difference is that `generalize' : e = x` also succeeds when `e`
does not occur in the goal. It is similar to `set`, but the resulting hypothesis `x` is not a local definition.
-/
meta def generalize' (h : parse ident?) (_ : parse $ tk ":") (p : parse generalize_arg_p) : tactic unit :=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably go into Lean 3.6.

@gebner gebner 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 Feb 21, 2020
@mergify mergify bot merged commit ffb6d6e into master Feb 21, 2020
@mergify mergify bot deleted the local_def_tactics branch February 21, 2020 22:47
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…-community#1953)

* feat(tactic): add various tactics about local definitions

* remove {α β}

* rename generalize' in monotonicity.

* updates after reviews

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 16, 2020
…-community#1953)

* feat(tactic): add various tactics about local definitions

* remove {α β}

* rename generalize' in monotonicity.

* updates after reviews

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

4 participants