-
Notifications
You must be signed in to change notification settings - Fork 435
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: refine how named arguments suppress explicit arguments #5283
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
github-actions
bot
added
the
toolchain-available
A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
label
Sep 8, 2024
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/batteries
that referenced
this pull request
Sep 8, 2024
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/mathlib4
that referenced
this pull request
Sep 8, 2024
leanprover-community-bot
added
the
breaks-mathlib
This is not necessarily a blocker for merging: but there needs to be a plan
label
Sep 8, 2024
Mathlib CI status (docs):
|
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/batteries
that referenced
this pull request
Sep 8, 2024
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/mathlib4
that referenced
this pull request
Sep 8, 2024
Mathlib CI status (docs):
|
kmill
changed the title
fix: improve elaboration of
fix: refine how named arguments suppress explicit arguments
Sep 24, 2024
_
s for inst implicit arguments in explicit @
mode
github-actions
bot
temporarily deployed
to
lean-lang.org/lean4/doc
September 24, 2024 07:54
Inactive
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/batteries
that referenced
this pull request
Sep 24, 2024
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/mathlib4
that referenced
this pull request
Sep 24, 2024
Mathlib CI status (docs):
|
github-actions
bot
temporarily deployed
to
lean-lang.org/lean4/doc
September 24, 2024 08:14
Inactive
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/batteries
that referenced
this pull request
Sep 24, 2024
Originally this PR conservatively changed how |
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/mathlib4
that referenced
this pull request
Sep 24, 2024
Mathlib CI status (docs):
|
kmill
force-pushed
the
explicit_inst_holes
branch
from
September 24, 2024 21:35
3fb427e
to
fe12c58
Compare
github-actions
bot
temporarily deployed
to
lean-lang.org/lean4/doc
September 24, 2024 22:01
Inactive
github-actions
bot
temporarily deployed
to
lean-lang.org/lean4/doc
September 24, 2024 22:10
Inactive
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/batteries
that referenced
this pull request
Sep 24, 2024
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/mathlib4
that referenced
this pull request
Sep 24, 2024
Mathlib CI status (docs):
|
github-actions
bot
temporarily deployed
to
lean-lang.org/lean4/doc
September 24, 2024 22:37
Inactive
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/batteries
that referenced
this pull request
Sep 24, 2024
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/mathlib4
that referenced
this pull request
Sep 24, 2024
Mathlib CI status (docs):
|
Mathlib CI status (docs):
|
leanprover-community-bot
added
builds-mathlib
CI has verified that Mathlib builds against this PR
and removed
breaks-mathlib
This is not necessarily a blocker for merging: but there needs to be a plan
labels
Sep 25, 2024
Mathlib CI status (docs):
|
kmill
commented
Sep 27, 2024
github-actions
bot
temporarily deployed
to
lean-lang.org/lean4/doc
September 27, 2024 19:10
Inactive
leanprover-community-mathlib4-bot
added a commit
to leanprover-community/batteries
that referenced
this pull request
Sep 27, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Sep 27, 2024
… `@` mode This PR does two things: 1. Recall that in explicit mode, a `_` in place of an instance implicit argument still does instance synthesis (one can use `(_)` instead to turn it into a true implicit argument). There has been missing terminfo for such arguments. Now hovering will display the synthesized expression and its type. 2. Recall that when there are named arguments, every explicit argument the named arguments depend on become implicit. This was interacting badly with instance arguments in explicit mode, since the `_` special casing mentioned above was not respecting this rule. Now it respects this rule. This PR is operating under the assumption that the named argument feature in item 2 is meant to be active in explicit mode. An alternative to item 2 would be to turn the feature off in explicit mode.
kmill
force-pushed
the
explicit_inst_holes
branch
from
September 27, 2024 19:47
fc93d91
to
c4f5ae9
Compare
github-actions
bot
temporarily deployed
to
lean-lang.org/lean4/doc
September 27, 2024 20:00
Inactive
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
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.
Note: Most of the implementation of this PR is at #5512 due to a bad rebase before merging.
Recall that currently named arguments suppress all explicit parameters that are dependencies. This PR limits this feature to only apply to true structure projections, except in the case where it is triggered when there are no more positional arguments. This preserves the primary reason for generalizing this feature (issue #1851), while removing the generalized feature, which has led to numerous confusions (issue #1867). This also fixes a bug pointed out on Zulip where in
@
mode, instance implicit parameter dependencies to named arguments would be suppressed unless the next positional argument was_
.More detail:
NamedArg
structure now has asuppressDeps : Bool
field. It is set totrue
for theself
argument in structure projections. If there is such aNamedArg
, explicit parameters that are dependencies to the named argument are turned into implicit arguments. The consequence is that all structure projections are treated as if their type parameters are implicit, even for class projections. This flag is not used for generalized field notation.rw [lem (h := foo)]
wherelem
has explicit arguments thath
depends on._
arguments register terminfo and are hoverable...
is respected in explicit mode.This implements RFC #5397. The
suppressDeps
flag suggests a future possibility of a named argument syntax that can suppress dependencies.