fix: ensure autoparam errors have correct positions #4926
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Autoparam tactic scripts have no source positions, which previously made it so that any errors or messages would be logged at the current ref, which was the application or structure instance being elaborated. However, with incrementality the ref is now carefully managed to avoid leakage of outside data. This prevents the elaborator's ref from being used as the tactic's ref, causing errors to be misplaced. To fix this, now the elaborators insert the ref's source positions everywhere into the autoparam tactic script. Closes leanprover#4880
github-actions
bot
added
the
toolchain-available
|
Mathlib CI status (docs):
|
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 17, 2024
This PR fixes a regression where goals that don't exist were being displayed. The regression was triggered by #5835 and originally caused by #4926. Bug originally reported at https://leanprover.zulipchat.com/#narrow/channel/270676-lean4/topic/tactic.20doesn't.20change.20primary.20goal.20state/near/488957772. The cause of this issue was that #5835 made certain `SourceInfo`s canonical, which was directly transferred to several `TacticInfo`s by #4926. The goal state selection mechanism would then pick up these extra `TacticInfo`s. The approach taken by this PR is to ensure that the `SourceInfo` that is being transferred by #4926 is noncanonical.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Autoparam tactic scripts have no source positions, which until recently made it so that any errors or messages would be logged at the current ref, which was the application or structure instance being elaborated. However, with the new incrementality features the ref is now carefully managed to avoid leakage of outside data. This inhibits the elaborator's ref from being used for the tactic's ref, causing messages to be placed at the beginning of the file rather than on the syntax that triggered the autoparam.
To fix this, now the elaborators insert the ref's source position everywhere into the autoparam tactic script.
If in the future messages for synthetic tactics appear at the tops of files in other contexts, we should consider an approach where
Lean.Elab.Term.withReuseContextuses something likereplaceRefto set the ref while disabling incrementality when the tactic does not contain source position information.Closes #4880