Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0007818OCamltypingpublic2018-07-10 17:362018-09-21 08:19
Reportermandrykin 
Assigned Togarrigue 
PriorityhighSeveritycrashReproducibilityalways
StatusresolvedResolutionfixed 
Platformx86_64 SMPOSLinux 4.4.0OS VersionUbuntu 16.04.4
Product Version4.06.1 
Target Version4.07.1+dev/rc1Fixed in Version4.07.1+dev/rc1 
Summary0007818: Seemingly unrelated and spurious error (unaliasable module) with value shadowing
Description# module Termsig = struct
    module Term0 = struct
      module type S = sig
        module Id : sig end
      end
    end
    module Term = struct
      module type S = sig
        module Term0 : Term0.S
        module T = Term0
      end
    end
  end;;
  
# module Make (T' : Termsig.Term.S) = struct
    module T = struct
      include T'.T
      let u = 1
      let u = 1
    end
  end;;

Gives:
Error: Signature mismatch:
       Modules do not match:
         sig
           module Id = T'.Term0.Id
           module Id2 = Id
           val u : int
           val u : int
         end
       is not included in
         sig module Id = T'.Term0.Id module Id2 = Id val u : int end
       In module Id:
       Module T'.Term0.Id cannot be aliased

While after removing the rebinding of `u`:

# module Make (T' : Termsig.Term.S) = struct
    module T = struct
      include T'.T
      let u = 1
    end
  end;;

is accepted as well as

# module Make (T' : Termsig.Term.S) = struct
    module T = struct
      include T'.T
      module Id2 = Id
      let u = 1
    end
  end;;

(so the module aliasing seems not to be related).

The real cause of the error is too confusing to me to try to explain it more clearly in the summary.
TagsNo tags attached.
Attached Files? file icon cannot_alias.ml [^] (601 bytes) 2018-07-10 17:36 [Show Content]
? file icon cannot_alias2.ml [^] (452 bytes) 2018-07-10 18:16 [Show Content]

- Relationships

-  Notes
(0019227)
mandrykin (reporter)
2018-07-10 18:21

Added another shorter reproduction case with another inconsistency: while the first functor is accepted, it's not possible to specify its (the same) output signature explicitly.
(0019228)
lpw25 (developer)
2018-07-10 18:22

This is a symptom of a bug that I already knew about but never got around to fixing (or reporting). Basically, the check that is supposed to stop you making aliases to functor parameters is very broken. IIRC there are at least 3 ways to work around it -- and you have stumbled onto one of them here.

I hadn't realised this could cause actual problems, but it seems that it can cause module inclusion checks to fail as in this case (although there is no explicit coercion the presence of repeated values causes an inclusion check between the module type with two `u`s and the module type with one `u`).

I was already planning to try and remove the restriction on aliases to functor parameters for 4.08, which would fix this bug as a side-effect.
(0019229)
mandrykin (reporter)
2018-07-10 18:45

So would you recommend currently to better avoid those aliases (to functor arguments) even when they are accepted e.g. by using

(* ... module Id : sig type t end ... *)

module Make3 (T' : Termsig.Term.S) = struct
  module T = struct
    include (T'.T : Termsig.Term0.S with module Id = T'.T.Id)
    module Id2 = Id
    let u = 1
    let u = 1
  end
end;; (* OK *)

(so that there are just type equalities instead of module aliases)? Module aliases are inferred by default even if the resulting signature doesn't contain them, so e.g.

module Make3 (T' : Termsig.Term.S) : sig end = struct
  module T = struct
    include T'.T
    module Id2 = Id
    let u = 1
    let u = 1
  end
end;;

also fails, but they can be avoided if prevented earlier (upon include).
(0019230)
lpw25 (developer)
2018-07-11 00:09

Yeah, that's probably your best bet for now.
(0019231)
garrigue (manager)
2018-07-11 10:28

Wouldn't it be reasonable to fix this bug (i.e. correctly detect non-aliasable modules)?
In this case, it looks like one should fix the ~aliasable argument to strengthen in env.ml, to reflect whether the path is alienable or not.
Even if you allow aliasing them at a later time, this cannot be a physical aliasing, so the distinction will still be relevant, isn't it?
(0019232)
garrigue (manager)
2018-07-11 10:45

Actually this doesn't seem to be the one. There are many calls to strengthen...
(0019233)
garrigue (manager)
2018-07-11 11:37

By the way, this is clearly a soundness bug. Using cannot_alias2.ml, we have:

# module M = Make1 (struct module Term0 = struct module Id = struct let x = "a" end end module T = Term0 end);;
module M : sig module Id : sig val x : string end module Id2 = Id end
# M.Id.x;;
Segmentation fault
(0019240)
garrigue (manager)
2018-07-12 03:13

I have opened https://github.com/ocaml/ocaml/pull/1898/ [^]
It proposes a way to fix this problem, by checking the signature of functor argument types.
This is an incompatible change, prohibiting all the functors you proposed, but since this is a soundness bug, I prefer being on the safe side.

Leo, if you know other ways to break Env.is_functor_arg, please speak up because this is a bad problem.

Comment: I thought originally that Env.is_functor_arg was safe, because all accesses to the contents of the argument should use a path which starts from it. Unfortunately, copying components of a submodule of the argument seem to lose this connection, allowing to use aliases coming from inside the argument. Properly tracking this seems complex, hence this somehow radical approach.
(0019371)
garrigue (manager)
2018-09-18 03:45

New alternative approach in https://github.com/ocaml/ocaml/pull/2051 [^]
The previous PR did break a lot of code, this one should be less intrusive.
(0019377)
garrigue (manager)
2018-09-21 08:19

Solved in 4.07.1 and trunk by GPR#2051, commits 457dcc5a and e8bb50e421+f4274ed89.
All the examples of the original report are now accepted (but with a different type), and the related unsoundness is fixed. However another unsoundness has been found in the process :-(

- Issue History
Date Modified Username Field Change
2018-07-10 17:36 mandrykin New Issue
2018-07-10 17:36 mandrykin File Added: cannot_alias.ml
2018-07-10 18:16 mandrykin File Added: cannot_alias2.ml
2018-07-10 18:21 mandrykin Note Added: 0019227
2018-07-10 18:22 lpw25 Note Added: 0019228
2018-07-10 18:45 mandrykin Note Added: 0019229
2018-07-11 00:09 lpw25 Note Added: 0019230
2018-07-11 10:28 garrigue Note Added: 0019231
2018-07-11 10:45 garrigue Note Added: 0019232
2018-07-11 11:37 garrigue Note Added: 0019233
2018-07-11 11:39 garrigue Assigned To => garrigue
2018-07-11 11:39 garrigue Priority low => high
2018-07-11 11:39 garrigue Severity minor => crash
2018-07-11 11:39 garrigue Status new => confirmed
2018-07-11 11:39 garrigue Target Version => 4.07.1+dev/rc1
2018-07-12 03:13 garrigue Note Added: 0019240
2018-09-18 03:45 garrigue Note Added: 0019371
2018-09-21 08:19 garrigue Note Added: 0019377
2018-09-21 08:19 garrigue Status confirmed => resolved
2018-09-21 08:19 garrigue Fixed in Version => 4.07.1+dev/rc1
2018-09-21 08:19 garrigue Resolution open => fixed


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker