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

coqchk reports names from inner modules of opaque modules as axioms #12845

Closed
JasonGross opened this issue Aug 17, 2020 · 4 comments · Fixed by #12862
Closed

coqchk reports names from inner modules of opaque modules as axioms #12845

JasonGross opened this issue Aug 17, 2020 · 4 comments · Fixed by #12862
Labels
part: checker The coqchk binary for validating .vo files.
Milestone

Comments

@JasonGross
Copy link
Member

Description of the problem

#5030 strikes again.

$ cat foo.v
Module Type A.
  Module B.
    Axiom t : Set.
  End B.
End A.

Module a : A.
  Module B.
    Definition t : Set := unit.
  End B.
End a.

Check a.B.t.
$ coqc -q -R . Top foo.v
a.B.t
     : Set
$ coqchk -silent -o -Q . Top foo.vo

CONTEXT SUMMARY
===============

* Theory: Set is predicative

* Axioms:
    Top.foo.a.B.t

* Constants/Inductives relying on type-in-type: <none>

* Constants/Inductives relying on unsafe (co)fixpoints: <none>

* Inductives whose positivity is assumed: <none>

By contrast:

$ cat bar.v
Module Type A.
  Module B.
    Axiom t : Set.
  End B.
End A.

Module a <: A.
  Module B.
    Definition t : Set := unit.
  End B.
End a.

Check a.B.t.
$ coqc -q -R . Top bar.v
a.B.t
     : Set
$ coqchk -silent -o -Q . Top bar.vo

CONTEXT SUMMARY
===============

* Theory: Set is predicative

* Axioms: <none>

* Constants/Inductives relying on type-in-type: <none>

* Constants/Inductives relying on unsafe (co)fixpoints: <none>

* Inductives whose positivity is assumed: <none>

cc @proux01 the author of #12076 and @ppedrot who seemed to be the main other person involved in the discussion

Coq Version

master / 8.13 / ef08abe

@JasonGross JasonGross added the part: checker The coqchk binary for validating .vo files. label Aug 17, 2020
@ppedrot
Copy link
Member

ppedrot commented Aug 17, 2020

Well, the criterion was known to be neither sound nor complete, so ¯_(ツ)_/¯

@JasonGross
Copy link
Member Author

Shouldn't it be possible to just have the code recurse over inner modules or something?

@JasonGross
Copy link
Member Author

Like, would it be enough to have

coq/checker/mod_checking.ml

Lines 103 to 112 in aa92642

let collect_constants_without_body sign mp =
let collect_sf s lab = function
| SFBconst cb ->
let c = Constant.make2 mp lab in
if Declareops.constant_has_body cb then s else Cset.add c s
| SFBmind _ | SFBmodule _ | SFBmodtype _ -> s in
match sign with
| MoreFunctor _ -> Cset.empty (* currently ignored *)
| NoFunctor struc ->
List.fold_left (fun s (lab,mb) -> collect_sf s lab mb) Cset.empty struc

call itself recursively when it hits SFBmodule?

@proux01
Copy link
Contributor

proux01 commented Aug 18, 2020

Like, would it be enough to have [...] call itself recursively when it hits SFBmodule?

Quite probably yes (feel free to open a PR).

However, as noticed by Pierre Marie, this would remain incomplete (I wouldn't bet for unsound but he certainly knows better than me). Having something complete seems feasible but requires some work which I didn't found time for yet (unlikely to change in a near future unfortunately). So, if you want to dig into this, feel free to, but the best I can promise is a careful review (maybe could I trade it for a review of #12218 ;-) ).
Thanks for reporting anyway!

JasonGross added a commit to JasonGross/fiat-crypto that referenced this issue Aug 19, 2020
By copying over the implementation of nsatz, we can drop the
dependencies on the real axioms.  Together with
coq/coq#12845, this should eliminate the last
axioms reported by coqchk, allowing us to be done with mit-plv#736 (maybe
modulo adding a CI job to check that no axioms creep back in).

Not clear to me if this is worth it...
@coqbot-app coqbot-app bot added this to the 8.13+beta1 milestone Aug 27, 2020
JasonGross added a commit to JasonGross/fiat-crypto that referenced this issue Sep 1, 2020
By copying over the implementation of nsatz, we can drop the
dependencies on the real axioms.  Together with
coq/coq#12845, this should eliminate the last
axioms reported by coqchk, allowing us to be done with mit-plv#736 (maybe
modulo adding a CI job to check that no axioms creep back in).

Not clear to me if this is worth it...
JasonGross added a commit to JasonGross/fiat-crypto that referenced this issue Sep 1, 2020
By copying over the implementation of nsatz, we can drop the
dependencies on the real axioms.  Together with
coq/coq#12845, this should eliminate the last
axioms reported by coqchk, allowing us to be done with mit-plv#736 (maybe
modulo adding a CI job to check that no axioms creep back in).

Not clear to me if this is worth it...
JasonGross added a commit to mit-plv/fiat-crypto that referenced this issue Sep 2, 2020
By copying over the implementation of nsatz, we can drop the
dependencies on the real axioms.  Together with
coq/coq#12845, this should eliminate the last
axioms reported by coqchk, allowing us to be done with #736 (maybe
modulo adding a CI job to check that no axioms creep back in).

Not clear to me if this is worth it...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part: checker The coqchk binary for validating .vo files.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants