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(frontends/lean/elaborator): don't block on metavariables that depend on an out_param #657

Closed
wants to merge 6 commits into from

Conversation

Vierkantor
Copy link
Collaborator

We noticed in mathlib#11128 some cases where parameters weren't being inferred, even though they were out_params to an instance: it turned out the instances themselves also weren't being inferred by the elaborator. This PR tweaks the logic in elaborator::ready_to_synthesize to match the out_param handling in type_context_old::preprocess_class.

I created a new function that determines the list of parameters that are either an out_param themselves, or depend on an out_param. The code is a bit different since preprocess_class updates the expression during the check. I left the previous check of preprocess_class in an assert so we can be sure the results are the same.

Zulip thread: https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/Instance.20parameters.20depending.20on.20.60out_params.60

…end on an `out_param`

We noticed in mathlib#11128 some cases where parameters weren't being inferred, even though they were `out_param`s to an instance: it turned out the instances themselves also weren't being inferred by the elaborator. This PR tweaks the logic in `elaborator::ready_to_synthesize` to match the `out_param` handling in `type_context_old::preprocess_class`.

I created a new function that determines the list of parameters that are either an `out_param` themselves, or depend on an `out_param`. The code is a bit different since `preprocess_class` updates the expression during the check. I left the previous check of `preprocess_class` in an `assert` so we can be sure the results are the same.

Zulip thread: https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/Instance.20parameters.20depending.20on.20.60out_params.60
src/library/type_context.cpp Outdated Show resolved Hide resolved
src/library/type_context.cpp Outdated Show resolved Hide resolved
src/library/type_context.cpp Outdated Show resolved Hide resolved
tests/lean/tc_out_param_deps.lean Outdated Show resolved Hide resolved
src/library/type_context.cpp Outdated Show resolved Hide resolved
Co-authored-by: Gabriel Ebner <gebner@gebner.org>
@gebner
Copy link
Member

gebner commented Jan 4, 2022

bors r+

bors bot pushed a commit that referenced this pull request Jan 4, 2022
…end on an `out_param` (#657)

We noticed in [mathlib#11128](leanprover-community/mathlib#11128) some cases where parameters weren't being inferred, even though they were `out_param`s to an instance: it turned out the instances themselves also weren't being inferred by the elaborator. This PR tweaks the logic in `elaborator::ready_to_synthesize` to match the `out_param` handling in `type_context_old::preprocess_class`.

I created a new function that determines the list of parameters that are either an `out_param` themselves, or depend on an `out_param`. The code is a bit different since `preprocess_class` updates the expression during the check. I left the previous check of `preprocess_class` in an `assert` so we can be sure the results are the same.

Zulip thread: https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/Instance.20parameters.20depending.20on.20.60out_params.60

Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
@bors
Copy link

bors bot commented Jan 4, 2022

@bors bors bot changed the title fix(frontends/lean/elaborator): don't block on metavariables that depend on an out_param [Merged by Bors] - fix(frontends/lean/elaborator): don't block on metavariables that depend on an out_param Jan 4, 2022
@bors bors bot closed this Jan 4, 2022
@bors bors bot deleted the tc_out_param_deps branch January 4, 2022 10:33
Vierkantor added a commit that referenced this pull request Jan 4, 2022
Vierkantor added a commit that referenced this pull request Jan 5, 2022
This ensures non-`out_param`s are not accidentally filled through unification
with the temporary metavariables. `out_param`s are still allowed to contain
metavariables that will be unified (see also #657).

I moved the `ready_to_synthesize` check from the elaborator to the type context
in order to access it from the simplifier.

Co-Authored-By: Gabriel Ebner <gebner@gebner.org>
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

2 participants