Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006510OCamlOCaml typingpublic2014-08-08 09:402014-08-09 00:15
Reporterfrisch 
Assigned Tolpw25 
PrioritylowSeverityminorReproducibilityhave not tried
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version 
Target VersionFixed in Version4.02.0+dev 
Summary0006510: Removal of shadowed constructors of extensible types can break the structure of definitions
DescriptionThere 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.
TagsNo tags attached.
Attached Filesdiff file icon patch_6510.diff [^] (2,569 bytes) 2014-08-08 09:57 [Show Content]

- Relationships

-  Notes
(0011992)
frisch (developer)
2014-08-08 10:00

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)?
(0011993)
lpw25 (developer)
2014-08-08 12:31

> 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.
(0011994)
lpw25 (developer)
2014-08-08 12:33

> I've attached a patch (very lightly tested)

The patch looks correct to me.
(0011995)
frisch (developer)
2014-08-08 14:15
edited on: 2014-08-08 14:18

Thanks for the review. The patch is committed to 4.02 (15071) and to trunk (15072).

(0011997)
shinwell (developer)
2014-08-09 00:14

Closing, since this is apparently fixed.

- Issue History
Date Modified Username Field Change
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


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker