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

agda/agda#7674 eta-expand fields in record expression #2546

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

andreasabel
Copy link
Member

@andreasabel andreasabel commented Jan 13, 2025

agda/agda#7674 eta-expand fields in record expression

Agda PR #7674 requires some implicit arguments explicitly in some record expression given since variance info got refined (invariant to non-variant) and hence solutions are no longer unique.

Fixing Agda to commit 2ffb002 now in agda/agda#7674.
If this PR is rebased or squashed (rather than merged with merge commit) the commit hash there needs to be updated accordingly.

Agda PR #7674 requires some implicit arguments explicitly in some record
expression given since variance info got refined (invariant to
non-variant) and hence solutions are no longer unique.

isMagma : IsMagma _≈_ _∙_
isMagma = record { isEquivalence = isEquivalence ; ∙-cong = ∙-cong }
isMagma = record { isEquivalence = isEquivalence ; ∙-cong = λ {y = y} {v = v} ∙-cong {y = y} {v = v} }
Copy link
Member Author

Choose a reason for hiding this comment

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

This checks with Agda master (2.8.0) but not with 2.7.0.1:

_x_48 ≈ y → _u_49 ≈ v → _x_48 ∙ _u_49 ≈ y ∙ v !=
{v = v₁ : Carrier} → x ≈ y → v ≈ v₁ → x ∙ v ≈ y ∙ v₁ because one is
an implicit function type and the other is an explicit function type
when checking that the expression
λ {y} {v} → ∙-cong {y = y} {v = v} has type Congruent₂ _≈_ _∙_

Apparently, in 2.8.0 we have fixed another bug but I could not find it now.

@jamesmckinna
Copy link
Contributor

jamesmckinna commented Jan 13, 2025

@andreasabel IIUC, as this is an upstream change to a not-yet-released agda-2.8.0, should this PR not be merged into targeted at the experimental branch of stdlib, and not at master?

@jamesmckinna jamesmckinna added this to the Agda v2.8.0 milestone Jan 13, 2025
@andreasabel
Copy link
Member Author

@andreasabel IIUC, as this is an upstream change to a not-yet-released agda-2.8.0, should this PR not be merged into targeted at the experimental branch of stdlib, and not at master?

Formally, yes.

However, in this case the change is in its present form compatible both with current Agda and 2.8.0, and it does not change the value of any definition, only makes some arguments explicit, so I thought we could just merge this into master.
Let me know if you think otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Changes induced by Agda upstream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants