Skip to content

Cleanup for overridden notations #232

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

Merged
merged 18 commits into from
Apr 12, 2025

Conversation

DmxLarchey
Copy link
Collaborator

@DmxLarchey DmxLarchey commented Jan 24, 2025

This PR is a follow up of #231. It intends to solve remaining issues related to overridden notations, ie warnings that pop up when compiling without the flag -notation-overridden. @dominik-kirst would you mind having a look? Orelse pinging the author of the corresponding files? I also ping @mrhaandi who initiated those warning issues in the master branch.

Notice that this branch is supposed to be compiled with rocq, not coq.

The concerned files are below.

Proposed solutions by @DmxLarchey :

  • FOL/FOL.v;
  • FOL/Utils/FriedmanTranslation.v;
  • FOL/Utils/FriedmanTranslationFragment.v;
  • FOL/Reductions/H10p_to_FA.v.

Proposed solutions by @mrhaandi :

  • FOL/binFST_undec.v;
  • FOL/Semantics/FiniteTarski/DoubleNegation.v;
  • FOL/Reductions/H10UPC_to_FOL_friedman.v;
  • FOL/Reductions/binZF_to_binFST.v;
  • FOL/Reductions/ZF_to_FST.v.

@mrhaandi
Copy link
Collaborator

DmxLarchey#16 corrects scoping issues in the Unsolved so far cases, leaving no warnings.

@mrhaandi
Copy link
Collaborator

The CI currently fails because rocq changed its package structure.

@DmxLarchey
Copy link
Collaborator Author

@dominik-kirst Could you please have a look at the proposed changes. They are all in the theories/FOL sub-dir and thus most likely code that you wrote yourself or participate in writing. Best, Dominique.

@mrhaandi
Copy link
Collaborator

@DmxLarchey could you integrate #236 into this PR?

@DmxLarchey
Copy link
Collaborator Author

@DmxLarchey could you integrate #236 into this PR?

Done

@JoJoDeveloping
Copy link
Contributor

Apart from that, what is missing for this PR? I not really sure what is the difference between the two proposed solutions from the PR description.. Which is this, where can I find the other one?

@mrhaandi
Copy link
Collaborator

These are not alternatives per se, but the combination of two distinct solutions for different files. @DmxLarchey put notations in Modules (as should be standard). This did not work for other files, which were incorrectly scoped.

@DmxLarchey
Copy link
Collaborator Author

Apart from that, what is missing for this PR? I not really sure what is the difference between the two proposed solutions from the PR description.. Which is this, where can I find the other one?

Nothing really but I did propose @dominik-kirst for a review. But we can merge w/o that review.

@dominik-kirst
Copy link
Collaborator

Looks all good to me, sorry for the delay!

@mrhaandi
Copy link
Collaborator

@DmxLarchey with @dominik-kirst's approval if it is fine with you, I can merge now.

@DmxLarchey
Copy link
Collaborator Author

@DmxLarchey with @dominik-kirst's approval if it is fine with you, I can merge now.

Thanks for doing this.

@mrhaandi mrhaandi merged commit 3edeccf into uds-psl:master Apr 12, 2025
1 check 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.

4 participants