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

Regression in ocamlc -pack typing #7932

Open
vicuna opened this Issue Feb 26, 2019 · 2 comments

Comments

Projects
None yet
1 participant
@vicuna
Copy link
Collaborator

commented Feb 26, 2019

Original bug ID: 7932
Reporter: @stedolan
Status: new
Resolution: open
Priority: normal
Severity: minor
Version: 4.08.0+dev/beta1/beta2
Category: typing
Monitored by: @nojb

Bug description

The package frama-c-base does not build with 4.08 or trunk, due to a change in the typing rules around ocamlc -pack.

I've reduced the issue to a minimal example:

==> Cint.ml <==
let is_cint_simplifier = []

==> Cint.mli <==
val is_cint_simplifier: Conditions.simplifier

==> Conditions.ml <==
type simplifier = Lang.pred list

==> Conditions.mli <==
type simplifier = Lang.pred list

==> Lang.ml <==
type pred = unit

==> Lang.mli <==
type pred

==> Pack.mli <==
module Lang : sig
  type pred
end
module Cint : sig
  val is_cint_simplifier: Conditions.simplifier
end
module Conditions : sig
  type simplifier = Lang.pred list
end

The command ocamlc -o Pack.cmo -pack Lang.cmo Cint.cmo Conditions.cmo gives the following error on 4.08 and trunk, but succeeds on earlier versions:

File "none", line 1:
Error: The implementation (obtained by packing)
does not match the interface Pack.mli:
In module Cint:
Modules do not match:
sig val is_cint_simplifier : Conditions.simplifier end
is not included in
sig val is_cint_simplifier : Conditions/2.simplifier end
In module Cint:
Values do not match:
val is_cint_simplifier : Conditions/1.simplifier
is not included in
val is_cint_simplifier : Conditions/2.simplifier
File "Pack.mli", line 5, characters 2-47: Expected declaration
File "Cint.mli", line 1, characters 0-45: Actual declaration
File "none", line 1:
Definition of module Conditions/1
File "none", line 1:
Definition of module Conditions/2

(The reproduction case is available as a gist, see below)

I suspect the change occurred during #2175. However, upon examining the test case, I think the new behaviour might be correct: Pack.mli has the modules in the wrong order, and indeed frama-c-base builds correctly when they are reordered. So it's possible that the only change needed is to change a '-' to a '*' in the Changes entry for #2175. But since I don't precisely understand what -pack is meant to do, more opinions would be welcome.

Steps to reproduce

git clone https://gist.github.com/d0137407e00ace3e7f83c4414e9adf52.git ocaml-pack-issue && ( cd ocaml-pack-issue; ./run.sh /path/to/ocamlc-408; )

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

Comment author: @lpw25

Yes, that looks like the new behaviour is correct. Previously the example will have had a reference to the unpacked [Conditions] module in the interface of the pack which is not what was intended and should not be allowed.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2019

Comment author: maro

Indeed, we noticed it in our development version and fixed it for the next Frama-C release.

In our tests, fixing the command line order seemed to fix it for 4.08 but prevented it from compiling in <4.08. So we decided to remove the cyclic dependency in the files, which was neither intended nor necessary.

@vicuna vicuna added the typing label Mar 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.