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

fix: hovers on binders with metavariables #4192

Merged
merged 2 commits into from
May 21, 2024
Merged

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented May 16, 2024

this fixes #4078. It is an alternative fix to the one in #4137, suggested
by @kmill.

Incidentially, it makes the unused variable linter better. My theory is that
if we don’t reset the info when backtracking, the binder shows up more than
once in the info tree, and then it is considered “used”, although there are
just multiple binders.

this fixes #4078

it is an alternative fix to the one in #4137, suggested by @kmill.
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label May 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 16, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc May 16, 2024 15:55 Inactive
@nomeata nomeata added the awaiting-review Waiting for someone to review the PR label May 16, 2024
@nomeata nomeata marked this pull request as ready for review May 16, 2024 15:55
@nomeata
Copy link
Contributor Author

nomeata commented May 16, 2024

The changes to the test output looks reasonable, in one case we see that the info tree no longer contains entries from a back-tracked attempt at elaboration.

@nomeata nomeata requested a review from kmill May 16, 2024 15:56
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 16, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented May 16, 2024

Mathlib CI status (docs):

@nomeata
Copy link
Contributor Author

nomeata commented May 16, 2024

Interesting, this makes the unused-variable-linter better (see leanprover-community/batteries#802)

nomeata added a commit to leanprover-community/mathlib4 that referenced this pull request May 16, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label May 16, 2024
Copy link
Collaborator

@kmill kmill left a comment

Choose a reason for hiding this comment

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

This looks good to me (auto implicits should act as if they were always there, right?), though it'd be good to get another opinion since I'm not completely sure why restore doesn't restore info trees to begin with.

@Kha
Copy link
Member

Kha commented May 21, 2024

I'm not completely sure why restore doesn't restore info trees to begin with.

I don't know. It might be good to double-check and document the need for this parameter but that shouldn't hold back this PR.

@Kha
Copy link
Member

Kha commented May 21, 2024

IIRC the original intent was to retain information from overloading but it seems some (all?) of that was switched to restoreInfo := true since then

@nomeata nomeata added this pull request to the merge queue May 21, 2024
Merged via the queue into master with commit 8240193 May 21, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review Waiting for someone to review the PR builds-mathlib CI has verified that Mathlib builds against this PR toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rpc Error when hovering variable
4 participants