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

Test PR related to Issue 228 #229

Merged
merged 12 commits into from
Jan 22, 2025
Merged

Conversation

DmxLarchey
Copy link
Collaborator

See #228

@DmxLarchey DmxLarchey marked this pull request as draft January 20, 2025 16:02
@DmxLarchey DmxLarchey mentioned this pull request Jan 20, 2025
@@ -539,7 +539,7 @@ Arguments vec_zero {n}.
Arguments vec_one {n}.

Reserved Notation " e '#>' x " (at level 58, format "e #> x").
Reserved Notation " e [ v / x ] " (at level 57, v at level 0, x at level 0,
Reserved Notation " e [ v / x ] " (at level 1, v at level 0, x at level 0,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The change of priority here (or else in file theories/MinskyMachines/MMenv/env.v) results in messing with the SSR syntax for rewrite, see eg file

by rewrite [LHS](vec_head_tail v) [in LHS](vec_head_tail (vec_tail v)) [in LHS](vec_0_nil (vec_tail (vec_tail v))).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it due to unscoped, global, top-level notations? If so, is it much work to change that to opt-in notations (a common design practice for modular code)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I modified the code to have a module vec_notations but it created some issues, see below. @mrhaandi does it suit you, and are the corrections it implied satisfactory for you ?

@DmxLarchey DmxLarchey requested a review from mrhaandi January 20, 2025 16:23
which is imported where needed. The Reserved Notations  _ #> _ and _ [ _ / _ ]
are not global anymore ...
@DmxLarchey
Copy link
Collaborator Author

The last warning issue remaining is related to
https://github.com/DmxLarchey/coq-library-undecidability/blob/7b03998b700bd304d8e0480881d0cb6f8f57e078/theories/MinskyMachines/MM2.v#L45

Reserved Notation "i '//' r '⇢' s" (at level 70, no associativity).
Reserved Notation "P '//' r '→' s" (at level 70, no associativity).
Reserved Notation "P '//' r '↠' s" (at level 70, no associativity).
Reserved Notation "P '//' r ↓" (at level 70, no associativity).

where Coq/Rocq asks for the reserved notation _ // _ ↓ to be put at level 1 (instead of 70), because it ends with a terminal symbol. However, doing so has the side effect of preempting other notations like _ // _ ⇢ _ which do not end with a terminal symbol. Hence, if they are not also at level 1, this mainly forbids the parsing of the other notations.

The reason I did choose level 70 for these is logical: these are Prop atoms and this level is nice for those kinds of terms.

@mrhaandi I do not know how to handle this at the moment?.

@DmxLarchey
Copy link
Collaborator Author

The last commit 10478ab solves the issue of left/right bi-closed notations that should be at level 0 according to Coq/Rocq. These had no side effects at all, so a straightforward correction.

@mrhaandi
Copy link
Collaborator

@DmxLarchey Is it correct that the last remaining issue is:

File "./Shared/Libs/DLW/Code/sss.v", line 25, characters 0-65:
Warning: Postfix notations (i.e. starting with a nonterminal symbol and
ending with a terminal symbol) should usually be at level 1 (default).
[postfix-notation-not-level-1,parsing,default]

@DmxLarchey
Copy link
Collaborator Author

@mrhaandi Yes this is correct except that the very same issue also pops up in:

Reserved Notation "i '//' r '⇢' s" (at level 70, no associativity).
Reserved Notation "P '//' r '→' s" (at level 70, no associativity).
Reserved Notation "P '//' r '↠' s" (at level 70, no associativity).
Reserved Notation "P '//' r ↓" (at level 70, no associativity).

One possibility could be to modify the notation for "P terminates on input t" to something like P ↓ t but it would be quite dissimilar to the other forms P // ....

@mrhaandi
Copy link
Collaborator

If the final one is too ugly / bothersome to fix, we can for the near future use the same local warning suppression as the Coq standard library:
https://github.com/coq/coq/blob/c1d8bee3b89b2fc9ba6c76f472983e0f2d194f75/theories/ssr/ssreflect.v#L98

… _ ↓" which is postfix

so expected at level 1 by Coq/Rocq
@DmxLarchey
Copy link
Collaborator Author

Thx for the advice @mrhaandi !!

@DmxLarchey
Copy link
Collaborator Author

@mrhaandi and @yforster If you are ok, I can merge this PR corresponding to issue #228

@DmxLarchey DmxLarchey added the enhancement New feature or request label Jan 22, 2025
@mrhaandi
Copy link
Collaborator

If you are ok, I can merge this PR corresponding to issue #228

Feel free to merge.
If you like, you can also inspect the -notation-incompatible-prefix warnings in your code (which currently are also suppressed) as part of another PR.

@DmxLarchey DmxLarchey marked this pull request as ready for review January 22, 2025 22:33
@DmxLarchey DmxLarchey merged commit 6eae942 into uds-psl:master Jan 22, 2025
2 checks passed
@DmxLarchey DmxLarchey deleted the issue_228 branch January 22, 2025 22:47
@DmxLarchey
Copy link
Collaborator Author

you can also inspect the -notation-incompatible-prefix warnings in your code (which currently are also suppressed) as part of another PR.

@mrhaandi task completed in PR #230

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants