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: only direct parents of classes create projections #5920

Merged
merged 5 commits into from
Nov 12, 2024

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented Nov 1, 2024

This PR changes the rule for which projections become instances. Before, all parents along with all indirect ancestors that were represented as subobject fields would have their projections become instances. Now only projections for direct parents become instances.

Features:

  • Only parents that are not ancestors of other parents get instances. This allows "discretionary" indirect parents to be inserted for the purpose of computing strict resolution orders when structure.strictResolutionOrder is enabled, without having an impact on typeclass synthesis.
  • Non-subobject projections are now theorems if the parent is a proposition. These are also no longer @[reducible].

Closes #2905

Before, all class ancestors that were representable as subobject fields had their projections be instances. This (1) didn't include all direct parents and (2) might include indirect parents. Now, all direct parent classes are instances.

Features:
- Only parents that are not ancestors of other parents get instances. This allows "discretionary" indirect parents to be inserted without having an impact on typeclass synthesis for the purpose of computing strict resolution orders when `structure.strictResolutionOrder` is enabled.
- Non-subobject projections are now theorems if the parent is a proposition.
- Parent projections are no longer added as instances for the `structure` command. Only the `class` command`.

Closes leanprover#2905
@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 Nov 1, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Nov 1, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 1, 2024
@leanprover-community-bot leanprover-community-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Nov 1, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Nov 1, 2024

Mathlib CI status (docs):

@kmill kmill changed the title feat: all direct parents of classes create projections feat: only direct parents of classes create projections Nov 10, 2024
@kmill kmill added the changelog-language Language features, tactics, and metaprograms label Nov 10, 2024
@leanprover-community-bot 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 Nov 10, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Nov 10, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 10, 2024
@kmill kmill enabled auto-merge November 12, 2024 00:17
@kmill kmill disabled auto-merge November 12, 2024 00:20
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Nov 12, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 12, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Nov 12, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 12, 2024
Comment on lines +33 to +36
/-!
Check that `A` is not an instance, since it is implied by the others.
-/
class E' extends C, D, A
Copy link
Contributor

Choose a reason for hiding this comment

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

This one surprises me a little; shouldn't it be an instance because the user asked for it in extends?

Copy link
Contributor

Choose a reason for hiding this comment

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

(unless this emits a warning, in which case I have no objection besides the test not checking for that warning)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, there's no warning. This feature is necessary for being able to satisfy the strict C3 linearization algorithm in some cases without affecting what the actual instances are. Currently strict mode is not the default, but I hope we can refactor mathlib at some point to have a strict linearization.

@kmill kmill added this pull request to the merge queue Nov 12, 2024
Merged via the queue into leanprover:master with commit 675d2d5 Nov 12, 2024
15 checks passed
@kmill kmill mentioned this pull request Nov 12, 2024
1 task
JovanGerb pushed a commit to JovanGerb/lean4 that referenced this pull request Jan 21, 2025
)

This PR changes the rule for which projections become instances. Before,
all parents along with all indirect ancestors that were represented as
subobject fields would have their projections become instances. Now only
projections for direct parents become instances.

Features:
- Only parents that are not ancestors of other parents get instances.
This allows "discretionary" indirect parents to be inserted for the
purpose of computing strict resolution orders when
`structure.strictResolutionOrder` is enabled, without having an impact
on typeclass synthesis.
- Non-subobject projections are now theorems if the parent is a
proposition. These are also no longer `@[reducible]`.

Closes leanprover#2905
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 changelog-language Language features, tactics, and metaprograms 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.

Non-direct class parents should not be instances
3 participants