Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006013OCamlOCaml generalpublic2013-05-10 23:512014-09-04 00:25
Reportersliquister 
Assigned To 
PrioritynormalSeverityminorReproducibilityalways
StatusconfirmedResolutionopen 
PlatformOSOS Version
Product Version4.00.1 
Target VersionundecidedFixed in Version 
Summary0006013: duplicate locations cause 'unused warnings' to be lost
DescriptionThe typer considers that either all the identifiers with a given name at a given location are used, or none or them are used.

One consequence is that warnings may not be given for the code generated by camlp4 extensions because the positions are usually duplicated. For instance if you write:

    type t = [ `a | `A ] with variants

the generated code will contain

    let fold ~a:a_fun ~a:a_fun ... = ... use a_fun twice ...

But no warning will be given. This is a regression compared to 3.12.
Steps To ReproduceThe easiest way to see the problem is using the toplevel:

$ ocaml -w +a
        OCaml version 4.00.1

# let f x x = x;; (* expected warning *)
Warning 27: unused variable x.
val f : 'a -> 'b -> 'b = <fun>
# let f x x = x;; (* wrong: no warning because it has already been given *)
val f : 'a -> 'b -> 'b = <fun>
# let f x x = x;; (* warning because of the positions are different *)
Warning 27: unused variable x.
val f : 'a -> 'b -> 'b = <fun>
#
TagsNo tags attached.
Attached Files

- Relationships
related to 0005787resolvedfrisch Bad behavior of 'Unused ...' warnings in the toplevel 

-  Notes
(0009269)
gasche (developer)
2013-05-11 00:43
edited on: 2013-05-11 00:44

Is the third declaration you give correct? On my machine under 4.00.1, I cannot "get a warning back" for the same argument name, or by changing the function name, I need to change the argument name:

# let f x x = x;;
Warning 27: unused variable x.
val f : 'a -> 'b -> 'b = <fun>
# let f x x = x;;
val f : 'a -> 'b -> 'b = <fun>
# let g x x = x;;
val g : 'a -> 'b -> 'b = <fun>
# let f y y = y;;
Warning 27: unused variable y.
val f : 'a -> 'b -> 'b = <fun>

Then, outside the toplevel (with a file compiled with `ocamlc -w +a -c foo.ml`), I cannot reproduce this behaviour. The following declaration, in the same file, each result in an unused message warning:

  let f x x = x
  let f x x = x
  let f ~x ~x = x


PS: I think we should *also* have a warning for twice-bound variables in an abstraction sequence. It's a remark by Kaustuv Chaudhuri that it's not right that (let f (x, x) = ...) fails while the currified (let f x x = ...) is silently accepted -- except for the eventual unused variable warning.

(0009270)
sliquister (reporter)
2013-05-11 02:30
edited on: 2013-05-11 23:44

The trick is that the third declaration has a space before the let, so all the positions are shifted by one. It was visible in the terminal but maybe there was a problem in the copy pasting.

It does not happen in ocamlc because the positions are different, since the field Lexing.pos_cnum of positions increases for instance.
If you create a camlp4 plugin that generates the code you wrote by hand with the same position for all the identifiers, you will have zero warnings.

I don't know about what you say in PS. Why would we want a warning for a mistake that already has a warning? The problem is not the lack of warnings, it's people not using them. If anything, what you say would make me think that [let f (x, x) = ...] could compile with an unused variable warning.

(0009274)
frisch (developer)
2013-05-13 15:00

> The easiest way to see the problem is using the toplevel:

This was reported in a different ticket (0005787) and has been fixed in the trunk.

> One consequence is that warnings may not be given for the code generated by camlp4 extensions because the positions are usually duplicated.

Indeed, the new implementation of the warning rely on the locations, which might not work very well with Camlp4.

A previous implementation of the new warnings relied on mutable markers inserted in the declaration records themselves, but this was considered bad style. This would have avoided the reported problem.

Instead of physical locations, we could use some logical occurrence path, but this would be quite heavy. A simpler fix could be to rely on physical equality of declaration records, but I need to think more about it.
(0009276)
gasche (developer)
2013-05-13 15:45

I think the "logical occurence path" idea is worth keeping on our mind, because it would solve other things too (eg. nail down the discussion of keeping locations in .cmi files, as those logical occurences are exactly what you need). That's one of the small-but-not-quite thing we would wish having an infinite supply of motivated interns for.

- Issue History
Date Modified Username Field Change
2013-05-10 23:51 sliquister New Issue
2013-05-11 00:43 gasche Note Added: 0009269
2013-05-11 00:44 gasche Note Edited: 0009269 View Revisions
2013-05-11 02:30 sliquister Note Added: 0009270
2013-05-11 23:44 sliquister Note Edited: 0009270 View Revisions
2013-05-13 15:00 frisch Note Added: 0009274
2013-05-13 15:45 gasche Note Added: 0009276
2013-05-17 18:40 gasche Status new => confirmed
2013-06-18 15:58 doligez Target Version => 4.02.0+dev
2013-06-18 15:58 doligez Relationship added related to 0005787
2013-07-12 18:15 doligez Target Version 4.02.0+dev => 4.01.1+dev
2014-05-25 20:20 doligez Target Version 4.01.1+dev => 4.02.0+dev
2014-07-24 23:05 doligez Target Version 4.02.0+dev => 4.02.1+dev
2014-09-04 00:25 doligez Target Version 4.02.1+dev => undecided


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker