|Anonymous | Login | Signup for a new account||2017-05-23 05:22 CEST|
|Main | My View | View Issues | Change Log | Roadmap|
|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0006510||OCaml||typing||public||2014-08-08 09:40||2016-12-07 11:34|
|Priority||low||Severity||minor||Reproducibility||have not tried|
|Target Version||Fixed in Version||4.02.0+dev|
|Summary||0006510: Removal of shadowed constructors of extensible types can break the structure of definitions|
|Description||There is some logic to drop constructors of extensible types when they are shadowed by a constructor of the same name later in the structure:|
# type t = ..;;
type t = ..
# module X = struct type t+= A type t+= X type t+= A end;;
module X : sig type t += X type t += A end
Individual constructors can also be dropped from a declaration containing multiple ones:
# module X = struct type t+= A|B type t+= X type t+= A end;;
module X : sig type t += B type t += X type t += A end
But because of the way declarations of multiple constructors in a single declaration is encoded in the internal representation of signatures, this can "merge" a constructor with a preceding declaration:
# module X = struct type t+= C type t+= A|B type t+= X type t+= A end;;
module X : sig type t += C | B type t += X type t += A end
This can be considered as a bug, since all the logic to keep a structured representation of multiple declarations is there to allow showing signatures that respect the original code. Here the constructors C and B were declared independently but they end up printed together.
But it is even worse, since the same applies if C is added to another type:
# type s = ..;;
type s = ..
# module X = struct type s+= C type t+= A|B type t+= X type t+= A end;;
module X : sig type s += C | B type t += X type t += A end
Here the printed signature is just plain wrong.
Note: this only applies for inferred signatures. Explicit signatures are probably supported.
|Tags||No tags attached.|
|Attached Files||patch_6510.diff [^] (2,569 bytes) 2014-08-08 09:57 [Show Content]|
|I've attached a patch (very lightly tested). I'm still wondering whether using a flat internal representation is the right approach. Leo: do you see major drawbacks in using a more structured representation (closer to the Parsetree/Typedtree structure)?|
> do you see major drawbacks in using a more structured representation (closer to the Parsetree/Typedtree structure)?
I think that it would make correct handling of ascription harder. In typing terms each extension constructor is a separate element of the module, so almost all code treats them independently. The only place where we care about how they are grouped is in printing.
> I've attached a patch (very lightly tested)
The patch looks correct to me.
edited on: 2014-08-08 14:18
Thanks for the review. The patch is committed to 4.02 (15071) and to trunk (15072).
|Closing, since this is apparently fixed.|
|2014-08-08 09:40||frisch||New Issue|
|2014-08-08 09:40||frisch||Description Updated||View Revisions|
|2014-08-08 09:45||frisch||Description Updated||View Revisions|
|2014-08-08 09:57||frisch||File Added: patch_6510.diff|
|2014-08-08 10:00||frisch||Note Added: 0011992|
|2014-08-08 12:31||lpw25||Note Added: 0011993|
|2014-08-08 12:33||lpw25||Note Added: 0011994|
|2014-08-08 14:15||frisch||Note Added: 0011995|
|2014-08-08 14:17||frisch||Note Edited: 0011995||View Revisions|
|2014-08-08 14:18||frisch||Note Edited: 0011995||View Revisions|
|2014-08-09 00:14||shinwell||Note Added: 0011997|
|2014-08-09 00:15||shinwell||Status||new => resolved|
|2014-08-09 00:15||shinwell||Fixed in Version||=> 4.02.0+dev|
|2014-08-09 00:15||shinwell||Resolution||open => fixed|
|2014-08-09 00:15||shinwell||Assigned To||=> shinwell|
|2014-08-09 00:15||shinwell||Assigned To||shinwell => lpw25|
|2014-08-09 00:15||shinwell||Status||resolved => assigned|
|2014-08-09 00:15||shinwell||Status||assigned => resolved|
|2016-12-07 11:34||xleroy||Status||resolved => closed|
|2017-02-23 16:45||doligez||Category||OCaml typing => typing|
|Copyright © 2000 - 2011 MantisBT Group|