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: apply’s error message should show implicit arguments as needed #3926

Closed
wants to merge 13 commits into from

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented Apr 16, 2024

luckily the necessary functionality already exists in the form of
addPPExplicitToExposeDiff. But it is not cheap, and we should not run this code
when the error message isn’t shown, so this used .ofPPFormat to assemble the error
message lazily.

Unfortunately, .ofPPFormat and FormatWithInfo have quite limited APIs for now.
This PR adds FormatWithInfo.append in a way that renumbers the keys in the Format
and the infos.

This also requires RBMap.mapKeysMono, and I got distracted a bit by writing the voluminous but boring proofs required for that somewhat trivial operation.

The lack of interpolation syntax for FormatWithInfo is also annoying. It would be really
nicer if one could “just” use MessageData in .ofPPFormat somehow.

Fixes #3232.

@nomeata nomeata changed the title joachim/failed to unify2 feat: apply’s error message should show implicit arguments as needed Apr 16, 2024
@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 Apr 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 16, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Apr 16, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

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

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 16, 2024
@nomeata
Copy link
Contributor Author

nomeata commented Apr 16, 2024

Probably #3929 is better.

@nomeata nomeata closed this Apr 16, 2024
github-merge-queue bot pushed a commit that referenced this pull request May 18, 2024
…3929)

luckily the necessary functionality already exists in the form of
`addPPExplicitToExposeDiff`. But it is not cheap, and we should not run
this code
when the error message isn’t shown, so we should do this lazily.

We already had `MessageData.ofPPFormat` to assemble the error message
lazily, but it
was restricted to returning `FormatWithInfo`, a data type that doesn’t
admit a nice
API to compose more complex messages (like `Format` or `MessageData`
has; an attempt to
fix that is in #3926).

Therefore we split the functionality of `.ofPPFormat` into
`.ofFormatWithInfo` and `.ofLazy`,
and use `.ofLazy` to compute the more complex error message of `apply`.

Fixes #3232.

---------

Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Co-authored-by: Wojciech Nawrocki <wjnawrocki@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

“failed to unify” message should not print identical types
2 participants