-
Notifications
You must be signed in to change notification settings - Fork 240
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
Parsing problems with the PermutationReasoning
combinators in Data.List.Relation.Binary.Permutation.Propositional
#2319
Comments
(UPDATED) Eg. I tried the following: -- Skip reasoning about the first two elements
step-swap′ : ∀ {x y} {xs ys zs : List A} ws → ws ≡ x ∷ y ∷ xs →
(y ∷ x ∷ ys) IsRelatedTo zs →
xs ↭ ys → ws IsRelatedTo zs
step-swap′ {x} {y} ws refl rel xs↭ys = ↭-go (swap x y xs↭ys) rel
Step-Swap : ∀ {ys zs} (ws : List A) → Set a
Step-Swap {ys} {zs} ws@(x ∷ y ∷ xs) = (y ∷ x ∷ ys) IsRelatedTo zs → xs ↭ ys → ws IsRelatedTo zs
Step-Swap {ys} {zs} ws@_ = ∀ {xs} → ws IsRelatedTo zs → xs ↭ ys → ws IsRelatedTo zs
step-swap : ∀ {ys zs} (ws : List A) → Step-Swap {ys} {zs} ws
step-swap ws@(x ∷ y ∷ xs) rel xs↭ys = step-swap′ ws refl rel xs↭ys
step-swap ws@(x ∷ []) rel _ = rel
step-swap ws@[] rel _ = rel
syntax step-swap ws ys↭zs xs↭ys = ws <<⟨ xs↭ys ⟩ ys↭zs which works for the first two existing examples in the second point of the OP above, but not the third! And the best I seem to be able to do for the lem₂ᵣ : 1 ∷ 2 ∷ 3 ∷ [] ↭ 3 ∷ 1 ∷ 2 ∷ []
lem₂ᵣ = begin
1 ∷ (2 ∷ 3 ∷ []) <⟨ swap 2 3 refl ⟩
1 ∷ 3 ∷ 2 ∷ [] <<⟨ refl ⟩
3 ∷ 1 ∷ 2 ∷ [] ∎ Is there some strictness problem behind the scenes? Eg in failing to force evaluation of UPDATED: the third example succeeds provided I bracket the initial term, suggesting some sort of evaluation/strictness problem? |
step-swap
combinator in Data.List.Relation.Binary.Permutation.Propositional
PermutationReasoning
combinators in Data.List.Relation.Binary.Permutation.Propositional
First thought: what's the precedence for all of these things? |
Well, not exactly sure what you intend by that, but... if you refactor a bunch of proofs cf. #2317 , and then try to add a new one/observe the behaviour of the old ones in the light of other refactoring, and you... don't get the behaviour you've come to expect, then .., is that not noteworthy? As to the matters at hand,
So: perhaps not a 'bug', but surely noteworthy? We don't have a But happy to close if |
You're legitimately confused because I wrote down the wrong word! Stupid fingers. I mean 'precedence' not 'precedent' !!! (Fixed now) It was meant to be a purely technical suggestion to look at the low level parsing machinery. I wonder if (ab)using 'syntax' notations is going to get into weird corners as how exactly it gets desugared / parsed might not be all that well documented? |
Thanks @JacquesCarette ! yes, I'm sure it is a low-level parsing thing, and/but despite my (ostensible!) credentials as a Computer Scientist (sic), I find that as a User, I simply can't hold the precedence tables in my head to figure ought what ought to happen to any given phrase, only to marvel when it works, and be frustrated when parsing is ambiguous, as seems to be the case here (which I am sure is to do with clashes in the Apologies for the tantrum-as-issue, but this kind of thing is... exasperating, and I realise I don't have the wherewithal to fix it on my own... boo-hoo! 😭 |
The cause of the first point is very simple. We don't simply don't define the symmetric version of |
In the second point, you can remove half the brackets:
but I agree you can't remove the other two pairs.... |
Okay so yes the fundamental problem is that the combinators don't interact right when you've got a real So I think these two operators are fundamentally broken as is. There are a few options:
To be honest I'd probably vote for option 3. To me Option 1 of changing the |
@MatthewDaggitt thanks for the penetrating analysis/diagnosis (of both problems; but the first is pure 🤦 for me), as well as a lucid account of possible ways to proceed. I definitely agree with you about options 1, and 2, and that's perhaps why I found myself experimenting with the Overall I think I'd be OK with deprecating the Meanwhile, who's going to handle the deprecation? (I think I've forgotten how to deprecate |
Yes as its equally broken.
It's not fiddly so much as impossible. We can and should attach a WARNING to the original definition used in the syntax, but last time I checked that doesn't cause it to fire when you use the syntax? Otherwise, we just say that the definition is deprecated and that people can't shouldn't use it. Less than ideal, but at some point in the future Agda might get updated so that we can attach the warnings to the syntax.... |
Now I've caught up with all of this, and I agree that syntax step-swap x y xs y↭z x↭y = x ∷ y ∷ xs <<⟨ x↭y ⟩ y↭z is fundamentally broken -- there is really no sane way for Agda to know what you mean here. One could consider replacing |
Re: "fundamentally broken", I agree, but I do nevertheless claim that the |
While I agree that type-checking is fine, I feel the extra brackets add severe levels of bafflement, as evidenced by your original question! I can easily imagine people in future playing with the same sort of expressions also reporting it as a bug... |
#2333 removes all uses of these combinators... apart from their (modest!) use in |
(micro-issue arising in the course of #2317 / #2311 )
UPDATED: two instances of what might be considered parsing 'bugs', but might merely be 'peeves'... but the first does seem to be a semantic failure of the current setup?
Suggest adding a comment in the definition of the combinators, or in the
README
to draw attention to this issue?Or is there a 'better' definition which might help avoid these problems?
master
, the reasoning step here makes a combination of aprep
/'prefix' step with a use ofshift
reflected symmetrically; but ifis replaced with
then the parser complains...
step-swap
combinator, defined as:seems to work in the handful of places in the library where it is used:
shift
drop-mid′
drop-mid′
but with some bracketing required, as well as only ever being used with
refl
as the intermediate step.But attempting to deploy it in the associated
README
leads to the following unpleasant need for re-bracketing when trying to re-express examplelem₂
using the combinator, as follows:Removing any of these brackets (or any other bracketing) seems to cause the parser to fall over.
The text was updated successfully, but these errors were encountered: