Skip to content

[Merged by Bors] - fix(ClickSuggestions): instantiate all metavariables in the local context#41001

Closed
JovanGerb wants to merge 1 commit into
leanprover-community:masterfrom
JovanGerb:Jovan-click_suggestions-mvars
Closed

[Merged by Bors] - fix(ClickSuggestions): instantiate all metavariables in the local context#41001
JovanGerb wants to merge 1 commit into
leanprover-community:masterfrom
JovanGerb:Jovan-click_suggestions-mvars

Conversation

@JovanGerb

@JovanGerb JovanGerb commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

This PR fixes the panic in #click_suggestions that was reported at https://leanprover.zulipchat.com/#narrow/channel/287929-mathlib4/topic/.23click_suggestions.20doesn.27t.20find.20lemma/near/606231763

There are two separate problems that this PR fixes:

  1. The implementation of #click_suggestion assumes all metavariables have already been instantiated. This is valid because they are all instantiated at the very beginning. However, this was not done properly, causing the types of free variables to still be able to contain metavariables.
  2. The function viewKAbstractSubExpr was able to reach its unreachable! block. The reason is that kabstractPositions calls instantiateMVars on the expression e but not on the pattern p, causing a discrepancy. This is fixed by removing the instantiateMVars call. This is fine, because in both of the use cases (rw?? and #click_suggestions), the expressions already have instantiated metavariables at this point.

Open in Gitpod

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

PR summary 85fabd70e2

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff (regex)

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## from your `mathlib4` directory:
git clone https://github.com/leanprover-community/mathlib-ci.git ../mathlib-ci

## summary with just the declaration names:
../mathlib-ci/scripts/pr_summary/declarations_diff.sh <optional_commit>

## more verbose report:
../mathlib-ci/scripts/pr_summary/declarations_diff.sh long <optional_commit>

The doc-module for scripts/pr_summary/declarations_diff.sh in the mathlib-ci repository contains some details about this script.

Declarations diff (Lean)

Lean-aware diff — post-build, computed from the Lean environment (commit 85fabd7).

  • +0 new declarations
  • −0 removed declarations

No declaration differences.


No changes to strong technical debt.

No changes to weak technical debt.

Current commit 85fabd70e2
Reference commit 9e46613652

This script lives in the mathlib-ci repository. To run it locally, from your mathlib4 directory:

git clone https://github.com/leanprover-community/mathlib-ci.git ../mathlib-ci
../mathlib-ci/scripts/reporting/technical-debt-metrics.sh pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).

@github-actions github-actions Bot added the t-meta Tactics, attributes or user commands label Jun 24, 2026

@joneugster joneugster left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maintainer delegate (my bad, didn't read the PR description carefully)

maintainer merge

Thanks for the PR!

@github-actions

Copy link
Copy Markdown

🚀 Pull request has been placed on the maintainer queue by joneugster.

@mathlib-triage mathlib-triage Bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Jun 25, 2026
@joneugster joneugster self-assigned this Jun 25, 2026
@fpvandoorn

Copy link
Copy Markdown
Member

Thanks!

bors merge

@mathlib-bors mathlib-bors Bot added the ready-to-merge This PR has been sent to bors. label Jun 26, 2026
@mathlib-triage mathlib-triage Bot removed the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Jun 26, 2026
mathlib-bors Bot pushed a commit that referenced this pull request Jun 26, 2026
…text (#41001)

This PR fixes the panic in `#click_suggestions` that was reported at https://leanprover.zulipchat.com/#narrow/channel/287929-mathlib4/topic/.23click_suggestions.20doesn.27t.20find.20lemma/near/606231763

There are two separate problems that this PR fixes:
1. The implementation of `#click_suggestion` assumes all metavariables have already been instantiated. This is valid because they are all instantiated at the very beginning. However, this was not done properly, causing the types of free variables to still be able to contain metavariables.
2. The function `viewKAbstractSubExpr` was able to reach its `unreachable!` block. The reason is that `kabstractPositions` calls `instantiateMVars` on the expression `e` but not on the pattern `p`, causing a discrepancy. This is fixed by removing the `instantiateMVars` call. This is fine, because in both of the use cases (`rw??` and `#click_suggestions`), the expressions already have instantiated metavariables at this point.
@mathlib-bors mathlib-bors Bot added the bors-staging This PR is currently being built by bors on the staging branch. label Jun 26, 2026
@mathlib-bors

mathlib-bors Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

@mathlib-bors mathlib-bors Bot changed the title fix(ClickSuggestions): instantiate all metavariables in the local context [Merged by Bors] - fix(ClickSuggestions): instantiate all metavariables in the local context Jun 26, 2026
@mathlib-bors mathlib-bors Bot closed this Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bors-staging This PR is currently being built by bors on the staging branch. ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants