Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve recursive module usage warnings #9393

Merged
merged 3 commits into from Apr 18, 2020

Conversation

lpw25
Copy link
Contributor

@lpw25 lpw25 commented Mar 24, 2020

Usage warnings are broken for recursive modules where a module or one of its components are only used in the signatures of other modules. For example,

module M : sig end = struct
  module rec Foo : sig
    type t
    val create : Bar.t -> t
  end = struct
    type t = unit

    let create _ = ()
  end

  and Bar : sig
    type t
  end = struct
    type t = unit
  end

  let _ = Foo.create
end;;
[%%expect{|
Line 14, characters 4-10:
14 |     type t
         ^^^^^^
Warning 34: unused type t.
Lines 13-17, characters 2-5:
13 | ..and Bar : sig
14 |     type t
15 |   end = struct
16 |     type t = unit
17 |   end
Warning 60: unused module Bar.
module M : sig end

This PR fixes the problem for the recursive modules themselves. The problem remains for the components of recursive modules. (i.e. the unused module warning above is fixed but the unused type warning is not).

Copy link
Contributor

@trefis trefis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

The location change that we can see in the tests seems fine.

typing/typemod.ml Show resolved Hide resolved
@lpw25 lpw25 force-pushed the improve-recmodule-usage-warnings branch from 162441c to 2c8f99e Compare April 18, 2020 07:11
@lpw25
Copy link
Contributor Author

lpw25 commented Apr 18, 2020

Rebased and changes entry added. Merging when CI is green.

@lpw25 lpw25 merged commit a40889d into ocaml:trunk Apr 18, 2020
gasche pushed a commit to gasche/ocaml that referenced this pull request Apr 18, 2020
…ings

Improve recursive module usage warnings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants