Skip to content

fix: make sure anonymous dot notation works with pi-type-valued type synonyms#4818

Merged
kmill merged 2 commits intoleanprover:masterfrom
kmill:fix_4761
Jul 24, 2024
Merged

fix: make sure anonymous dot notation works with pi-type-valued type synonyms#4818
kmill merged 2 commits intoleanprover:masterfrom
kmill:fix_4761

Conversation

@kmill
Copy link
Copy Markdown
Collaborator

@kmill kmill commented Jul 24, 2024

When resolving anonymous dot notation (.ident x y z), it would reduce the expected type to whnf. Now, it unfolds definitions step-by-step, even if the type synonym is for a pi type like so

def Foo : Prop := ∀ a : Nat, a = a
protected theorem Foo.intro : Foo := sorry
example : Foo := .intro

Closes #4761

When resolving anonymous dot notation (`.ident x y z`), it would reduce the expected type to whnf. Now, it unfolds definitions step-by-step, even if the type synonym is for a pi type like so
```lean
def Foo : Prop := ∀ a : Nat, a = a
protected theorem Foo.intro : Foo := sorry
example : Foo := .intro
```
@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 Jul 24, 2024
ghost pushed a commit to leanprover-community/batteries that referenced this pull request Jul 24, 2024
ghost pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jul 24, 2024
@ghost ghost added the builds-mathlib CI has verified that Mathlib builds against this PR label Jul 24, 2024
@ghost
Copy link
Copy Markdown

ghost commented Jul 24, 2024

Mathlib CI status (docs):

@kmill kmill changed the title fix: make sure anonymous dot notation works with type synonyms fix: make sure anonymous dot notation works with pi-type-valued type synonyms Jul 24, 2024
@kmill kmill enabled auto-merge July 24, 2024 16:58
tryPostponeIfNoneOrMVar expectedType?
let some expectedType := expectedType?
| throwError "invalid dotted identifier notation, expected type must be known"
forallTelescopeReducing expectedType fun _ resultType => do
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just curious: Could we have used forallTelescope if we had a TransparencyMode none?

Or would using withReducible do the right thing here! Or do we want people to use dot notation with reducible terms (I guess we do, commonly with abbrev, right?)

Copy link
Copy Markdown
Collaborator Author

@kmill kmill Jul 24, 2024

Choose a reason for hiding this comment

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

This made it mirroring how Lean.Elab.Term.consumeImplicits works (for x.f dot notation resolution), which also uses whnfCore and manual definition unfolding. It would be nice to have a version of forallTelescope where you can supply your own reduction function — that's something I was thinking about, but didn't want to touch that core meta code for this.

Supposing reducible transparency was ok here, a problem with withReducible is that it applies to the entire computation (plus the continuation!), where we just want to delimit it to the internal mechanism of forallTelescopeReducing.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, two continuation-style functions don't compose well. Could be a ReducibilityMode option to forallTelescopeReducing. (Not important)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, it could be, but better would be reduction function rather than configuration, since we do want whnfCore here rather than whnf. Without measuring, I worry a bit about the performance impact though since the function won't be specialized to whnf, and forallTelescope(Reducing) is used everywhere.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

On second thought, there's no need to adjust forallTelescopeReducingAuxAux itself, since you can use the pattern in this PR. I don't think there are really any optimizations from inserting a custom whnf function into it.

(This PR also continues on with whnfCore of the body, vs forallTelescopeReducing which gives you the unreduced body if it didn't reduce to a forall.)

@kmill kmill added this pull request to the merge queue Jul 24, 2024
ghost pushed a commit to leanprover-community/batteries that referenced this pull request Jul 24, 2024
ghost pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jul 24, 2024
Merged via the queue into leanprover:master with commit c545e7b Jul 24, 2024
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.

Anonymous dot notation fails on propositions defined by

2 participants