Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0007443OCamltypingpublic2016-12-28 01:352017-05-08 16:19
Reportergasche 
Assigned Tooctachron 
PrioritynormalSeveritytweakReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version4.05.0+dev 
Target VersionFixed in Version4.05.0+dev 
Summary0007443: Doubtful "unused open" warning on module used for record field disambiguation
DescriptionThe following code fails to compile with "-w @a" (setting all warnings as errors):

  module M = struct
    type t = { x : int; y : int }
  end

  let sum : M.t -> int = function
    | M.{ x; y } -> x + y

The compiler complains that the opening "M." in the pattern M.{ x; y } is unused. But if I remove "M.", with the warning-as-errors setting, I still get a compilation error!

I believe that access to a module record field for disambiguation purposes should be counted as a "use" of this module. One might argue that this needs not be the case if neither warnings 40 or 42 are set, but I would personally still count it as a "use" in those cases -- I want to be able to write code that respects the 40-as-an-error principle even if 40 is not an error or even not set.
Additional InformationThis issue can easily be worked around by writing the pattern { M.x; y } instead, but I personally find M.{ x; y } more elegant and, beside, the warning behavior may surprise innocent bystanders -- hence it is arguably a usability bug.
TagsNo tags attached.
Attached Files

- Relationships
related to 0007386acknowledged In or-patterns, propagate path disambiguation from one prefixed constructor to other constructors 

-  Notes
(0017048)
gasche (developer)
2016-12-28 02:48

I just noticed (in real code) that this also happens with variant constructor disambiguation:

  module M = struct
    type t = A | B | C
  end

  let sum : M.t -> unit = function
    | M.(A | B | C) -> ()

Again the compiler complains that "M." is unused. This is again wrong and, furthermore, there is no workaround as in the record case -- at least as long as 7386 is not resolved.
(0017053)
octachron (developer)
2016-12-28 17:59

I fear that this nothing but a simple mistake on my side when introducing pattern open: see the short fix in https://github.com/ocaml/ocaml/pull/990 [^] .
(0017054)
gasche (developer)
2016-12-29 20:08

I can confirm that the fix by octachron in GPR#990 fixed the problem -- I tested those two examples.

I should have realized that this was not an issue with "open" in general, but with the new pattern-open feature, as "let open M in function A | B | C -> ()" does not warn -- I failed to try.

Thanks octachron for the quick fix and the nice feature.
(0017782)
lpw25 (developer)
2017-05-08 16:17

This fix doesn't look correct to me. It looks as though it just turns off the unused open warning for opens in patterns, which is not the desired behaviour.
(0017783)
lpw25 (developer)
2017-05-08 16:19
edited on: 2017-05-08 16:19

Actually, nevermind. I see that the change is to turn off the warning during the `contains_gadt` check. Sorry for the noise.


- Issue History
Date Modified Username Field Change
2016-12-28 01:35 gasche New Issue
2016-12-28 02:48 gasche Note Added: 0017048
2016-12-28 02:48 gasche Relationship added related to 0007386
2016-12-28 17:59 octachron Note Added: 0017053
2016-12-29 20:08 gasche Note Added: 0017054
2016-12-29 20:08 gasche Status new => resolved
2016-12-29 20:08 gasche Fixed in Version => 4.05.0+dev
2016-12-29 20:08 gasche Resolution open => fixed
2016-12-29 20:08 gasche Assigned To => octachron
2017-02-23 16:45 doligez Category OCaml typing => typing
2017-05-08 16:17 lpw25 Note Added: 0017782
2017-05-08 16:19 lpw25 Note Added: 0017783
2017-05-08 16:19 lpw25 Note Edited: 0017783 View Revisions


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker