[ refactor ] v2.4-admissible version of #2952#2980
[ refactor ] v2.4-admissible version of #2952#2980jamesmckinna wants to merge 28 commits intoagda:masterfrom
Conversation
|
Gonna do @gallais 's trick of close and re-open to kick-start rechecking the various haskell failing versions? |
|
187 files, yikes. I've put that on my next |
No, you did the bulk of this already on #2952 this PR is based off that one but doesn't make the breaking change of fieldname... |
|
Unfortunately github can't seem to see that, so is asking me to review all of them again. But now that I know, I might be able to figure out what exactly is new here. |
Apologies. This doubtless down to my clumsy use of hit and GitHub. I'd had the thought that having a v2.4 version would get the worst of the use changes out of the way, (and that you'd already surveyed), and the rename+deprecate cycle proper can begin in the v3.x product line... |
|
|
||
| lookup : ∀ {counted n} → counted ⊕ n → ∀ x → Dec (x ∈ counted) | ||
| lookup {counted} _ x = Any.any? (_≟_ x) counted | ||
| lookup {counted} _ x = x ∈? counted |
There was a problem hiding this comment.
There's a subtle issue with this change: in the definition of _∈?_, the needle is on the left of the _≟_/_≈?_, but the old version you're replacing here has the needle on the right of the _≟_. I think this is an OK change to make, and being more consistent with the rest of the library is a worthy goal, but this should be a change we're making deliberately
| module Algebra.Construct.LexProduct | ||
| {ℓ₁ ℓ₂ ℓ₃ ℓ₄} (M : Magma ℓ₁ ℓ₂) (N : Magma ℓ₃ ℓ₄) | ||
| (_≟₁_ : Decidable (Magma._≈_ M)) | ||
| (_≈₁?_ : Decidable (Magma._≈_ M)) |
There was a problem hiding this comment.
Why did we use 1 as an index in the name for the decision procedure for a non-indexed relation?
| @@ -87,7 +87,7 @@ isDecEquivalence = ≡.isDecEquivalence _≟_ | |||
|
|
|||
| infix 4 _==_ | |||
| _==_ : Char → Char → Bool | |||
There was a problem hiding this comment.
This is called _≡ᵇ_ in Data.Nat.Base. We should probably fix that (maybe in a different PR?)
Cf. #2952 (comment)
NB. This is based off #2952 but without the fieldame change, so any review comments there should carry over here, mutatis mutandis...
This PR attacks the first part of #2845:
_≈?_for (instances of)DecidableEquality, as an alias for the field name_≟_public, but now uses it nowhere except to construct newRelation.Binary.Structures_≟_to_≈?_inIsDecEquivalence#2952 in using the new name everywhere elseCHANGELOGas to what is to comeFurther: (and this could form part of the husk of a now hollwed-out #2952 )
_≟_Aim:
Comments welcome!