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

fix: respect type reducibility when generating injectivity lemmas #2276

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

eric-wieser
Copy link
Contributor

@eric-wieser eric-wieser commented Jun 19, 2023

The relevant test case is:

def WrappedNat (_n : Nat) := Nat

structure WithWrapped : Type :=
(n : Nat)
(m : WrappedNat n)

#check WithWrapped.mk.injEq
#print WithWrapped.noConfusionType

The diff due to this PR is intended to be

 WithWrapped.mk.injEq (n : Nat) (m : WrappedNat n) (n : Nat) (m : WrappedNat n) :
-  ({ n := n, m := m } = { n := n, m := m }) = (n = n ∧ m = m)
+  ({ n := n, m := m } = { n := n, m := m }) = (n = n ∧ HEq m m)
@[reducible] protected def WithWrapped.noConfusionType.{u} : Sort u → WithWrapped → WithWrapped → Sort u :=
-fun P v1 v2 => WithWrapped.casesOn v1 fun n m => WithWrapped.casesOn v2 fun n_1 m_1 => (n = n_1 → m = m_1 → P) → P
+fun P v1 v2 => WithWrapped.casesOn v1 fun n m => WithWrapped.casesOn v2 fun n_1 m_1 => (n = n_1 → HEq m m_1 → P) → P

This forward-ports leanprover-community/lean#812.

For now, I am just creating this to see if it passes CI, since CI appears not to run on my branch in a fork.

@leodemoura leodemoura added the awaiting-author Waiting for PR author to address issues label Jun 21, 2023
@Kha
Copy link
Member

Kha commented Jul 19, 2023

The CI failure seems to be a simple matter of reordered messages; do we agree on the general change?

@Kha Kha added the awaiting-review Waiting for someone to review the PR label Jul 19, 2023
@leodemoura
Copy link
Member

The CI failure seems to be a simple matter of reordered messages.

I don't look at PRs until it is green. We should make it clear to all PR submitters that this is a requirement.

do we agree on the general change?

The change is not properly motivated. A link to another thread is not an acceptable. We should also make it clear on the contribution guidelines that a clear and short motivation for unsolicited PRs is a requirement.

@Kha Kha added the missing RFC Non trivial PR was submitted before RFC has been created label Sep 6, 2023
@github-actions github-actions bot added the stale label Oct 20, 2023
@github-actions github-actions bot removed the stale label Nov 2, 2023
@github-actions github-actions bot added the stale label Dec 21, 2023
@github-actions github-actions bot removed the stale label Feb 10, 2024
@@ -101,6 +101,7 @@ static optional<environment> mk_no_confusion_type(environment const & env, name
expr rhs_type = lctx.get_type(rhs);
level l = sort_level(type_checker(env, lctx).ensure_type(lhs_type));
expr h_type;
// TODO: this should respect reducibility
Copy link
Contributor Author

@eric-wieser eric-wieser Feb 10, 2024

Choose a reason for hiding this comment

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

In lean 3, this was possible with type_context_old ctx(env, transparency_mode::Reducible);; but type_context_old no longer exists.

I think the issue here is that mk_no_confusion_type is using the kernel, when it should be using the elaborator; but fixing that is a large refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author Waiting for PR author to address issues awaiting-review Waiting for someone to review the PR missing RFC Non trivial PR was submitted before RFC has been created stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants