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

Update the ssr examples #19

Closed
wants to merge 1 commit into from
Closed

Update the ssr examples #19

wants to merge 1 commit into from

Conversation

co-dan
Copy link
Collaborator

@co-dan co-dan commented Dec 5, 2020

Let me know what you think about these changes, Ralf.

@RalfJung
Copy link
Collaborator

RalfJung commented Dec 6, 2020

I don't understand any of this code so I don't really have an opinion.^^ So, seems fine to me.

However, before landing:

  • The README should be adjusted to remove the mention of ssreflect
  • We should put the example on CI to make sure this actually works on all supported Coq examples. This will be easy once fix Coq 8.10 warnings and build examples on CI #16 lands and you rebase your PR on top of that.

@palmskog
Copy link
Member

palmskog commented Dec 7, 2020

@co-dan @RalfJung this PR is a bit sad to me to read since the main task it accomplishes is to "de-depend" on coq-mathcomp-ssreflect. What is the main reason for removing this optional dependency? Recall that we can easily set up an opam file for the ssreflect examples which uses the right dependency at very little cost in CI.

Moreover, most SSReflect users in fact prefer to use the notations and definitions from coq-mathcomp-ssreflect rather than those in the Stdlib, so in my view the examples become less useful with this PR rather than more.

@co-dan
Copy link
Collaborator Author

co-dan commented Dec 7, 2020

@palmskog maybe it is better to make the old code work with the newer versions of ssreflect,
but I am not sure how to do this, due to the conflict between notations from mathcomp-ssreflect and autosubst

Moreover, most SSReflect users in fact prefer to use the notations and definitions from coq-mathcomp-ssreflect rather than those in the Stdlib, so in my view the examples become less useful with this PR rather than more.

you mean like using seq instead of list and so on?

@palmskog
Copy link
Member

palmskog commented Dec 7, 2020

you mean like using seq instead of list and so on?

Yes, like +1 for successors. In some cases, the new notations do not map to Stdlib concepts, so the semantics may change when notations are changed.

@RalfJung
Copy link
Collaborator

RalfJung commented Dec 8, 2020

Closing in favor of #20.

@RalfJung RalfJung closed this Dec 8, 2020
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

3 participants