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

improve commute-subst-rename #1006

Merged
merged 1 commit into from
Jun 15, 2024
Merged

Conversation

damhiya
Copy link
Contributor

@damhiya damhiya commented Jun 15, 2024

The commute-subst-rename lemma in the Substitution chapter quantifies over polymorphic renaming ρ : ∀{Γᵨ} → Rename Γᵨ (Γᵨ , ★) (i.e. the ρ itself quantifies over the context Γᵨ), but the parameter Γᵨ is only instantiated with Γᵨ = Γ and Γᵨ = Δ. Thus we can replace this with just two renamings ρ₁ : Rename Γ (Γ , ★) and ρ₂ : Rename Δ (Δ , ★). By this way, we can also avoid defining some what awkward ρ′. I think the resulting proof is more clean.

I also fixed the corresponding prose below the agda code. It seems that the equational reasoning part was just broken, for example in this line:

          ≡ rename S_ (exts σ (ρ y))
          ≡ rename S_ (rename S_ (σ (ρ y)      (by the premise)

which should be

          ≡ rename S_ (exts σ (ρ y))
          ≡ rename S_ (rename ρ (σ y))         (by the premise)

even if we keep using polymorphic renaming.

Copy link
Member

@wadler wadler left a comment

Choose a reason for hiding this comment

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

Thank you, SoonWon. I'm not sure how to judge the difference between the two proofs. Yours is more general, which is a plus, but the single polymorphic rename may be closer to how one should think of the situation. Since @jsiek wrote this chapter, he should decide. (And an extra thanks for correcting the prose, which is important in any case.)

Copy link
Collaborator

@jsiek jsiek left a comment

Choose a reason for hiding this comment

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

Thanks, these changes look good to me!

@wenkokke wenkokke added this pull request to the merge queue Jun 15, 2024
Merged via the queue into plfa:dev with commit fade166 Jun 15, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants