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

Add an -unused-type-warnings flag to the driver #493

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
unreleased
----------

- Add `-unused-type-warnings` flag to the driver to allow users to disable
the generation of warning 34 silencing structure items when using
`[@@deriving ...]` on type declarations. (#493, @NathanReb)

- Fix `Longident.parse` so it also handles indexing operators such as
`.!()`, `.%(;..)<-`, or `Vec.(.%())` (#494, @octachron)

Expand Down
18 changes: 16 additions & 2 deletions doc/driver.mld
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,22 @@ driver.exe [extra_args] [<files>]
-no-merge Do not merge context free transformations (better for debugging rewriters). As a result, the context-free transformations are not all applied before all impl and intf.
-cookie NAME=EXPR Set the cookie NAME to EXPR
--cookie Same as -cookie
-help Display this list of options
--help Display this list of options
-deriving-keep-w32 {impl|intf|both}
Do not try to disable warning 32 for the generated code
-deriving-disable-w32-method {code|attribute}
How to disable warning 32 for the generated code
-type-conv-keep-w32 {impl|intf|both}
Deprecated, use -deriving-keep-w32
-type-conv-w32 {code|attribute}
Deprecated, use -deriving-disable-w32-method
-deriving-keep-w60 {impl|intf|both}
Do not try to disable warning 60 for the generated code
-unused-code-warnings _ Allow ppx derivers to enable unused code warnings
-unused-type-warnings {true|false}
Allow unused type warnings for types with [@@deriving ...]
-help Display this list of options
--help Display this list of options

v}
{%html: </details>%}

Expand Down
11 changes: 10 additions & 1 deletion src/deriving.ml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ let () =
~doc:"_ Allow ppx derivers to enable unused code warnings"

let allow_unused_code_warnings () = !allow_unused_code_warnings
let allow_unused_type_warnings = ref Options.default_allow_unused_type_warnings

let () =
Driver.add_arg "-unused-type-warnings"
(Bool (( := ) allow_unused_type_warnings))
~doc:
"{true|false} Allow unused type warnings for types with [@@deriving ...]"

let allow_unused_type_warnings () = !allow_unused_type_warnings

module Args = struct
include (
Expand Down Expand Up @@ -701,7 +710,7 @@ let wrap_sig ~loc ~hide list =
+-----------------------------------------------------------------+ *)

let types_used_by_deriving (tds : type_declaration list) : structure_item list =
if keep_w32_impl () then []
if keep_w32_impl () || allow_unused_type_warnings () then []
else
List.map tds ~f:(fun td ->
let typ = Common.core_type_of_type_declaration td in
Expand Down
1 change: 1 addition & 0 deletions src/options.ml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
let default_allow_unused_code_warnings = false
let default_allow_unused_type_warnings = false
let perform_checks = false

(* The checks on extensions are only to get better error messages
Expand Down
63 changes: 59 additions & 4 deletions test/deriving_warning/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ triggering warnings, we also added optional arguments to `Deriving.make` so that
the ppx themselves can declare whether they need the driver to disable warnings
or not.

The following tests describe the behaviour of flags and features used to control
the emission of such warning silencing features.

One such flag and optional argument pair is the `-unused-code-warnings` flag and
`?unused_code_warning` `Deriving.V2.make` argument. Both of those default to
`false` and control whether we disable warnings 32 and 60 for generated code
Expand All @@ -26,7 +29,7 @@ We have a driver with 4 derivers linked in:
- two_do_warn
- alias_warn

----------------------------------------
--------------------------------------------------------------------------------

Let's consider the following ocaml source file using the zero_do_warn deriver

Expand Down Expand Up @@ -96,7 +99,7 @@ and compare the result of both driver invocations:
"@inline"]
[@@merlin.hide ]

-----------------------------------------------
--------------------------------------------------------------------------------

The default value of the -unused-code-warnings should be false:

Expand All @@ -116,7 +119,7 @@ The default value of the -unused-code-warnings should be false:
As we can see here, the warnings were disabled by the driver, as is expected
with -unused-code-warnings=false.

-----------------------------------------------
--------------------------------------------------------------------------------

Let's consider the following ocaml source file using the one_no_warn deriver

Expand Down Expand Up @@ -177,7 +180,7 @@ Same goes for .mli files:
val one : Zero.t One.t
end[@@ocaml.doc "@inline"][@@merlin.hide ]

-------------------------------------------------
--------------------------------------------------------------------------------

The alias_warn deriver is in fact an alias for two derivers:
- alias_do_warn, which is registered with unused_code_warnings=true
Expand Down Expand Up @@ -235,3 +238,55 @@ Same goes for .mli:
include sig [@@@ocaml.warning "-32"] val unit_two : unit end[@@ocaml.doc
"@inline"]
[@@merlin.hide ]

--------------------------------------------------------------------------------

Whenever a set of types has a [@@deriving ...] attached, ppxlib's driver always
generate structure items meant to disable unused type warnings (warning 34) for
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
generate structure items meant to disable unused type warnings (warning 34) for
generates structure items meant to disable unused type warnings (warning 34) for

any of those types.

Let's consider the following piece of OCaml code:

$ cat > unused_types.ml << EOF
> type t = int
> and u = string
> [@@deriving zero_do_warn]
> EOF

If we run the driver:

$ ./driver.exe -impl unused_types.ml
type t = int
and u = string[@@deriving zero_do_warn]
include struct let _ = fun (_ : t) -> ()
let _ = fun (_ : u) -> () end[@@ocaml.doc "@inline"][@@merlin.hide
]
include
struct
[@@@ocaml.warning "-60"]
module Zero = struct type t =
| T0 end
let zero = Zero.T0
let _ = zero
end[@@ocaml.doc "@inline"][@@merlin.hide ]

We can see that the driver generated two `let _ = fun (_ : ...`, one for each type
in the set.

We have a flag that disable this behaviour and allows unused type warnings to be
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
We have a flag that disable this behaviour and allows unused type warnings to be
We have a flag that disables this behaviour and allows unused type warnings to be

reported properly. Passing that flag to the driver should remove the two previously
mentionned items.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
mentionned items.
mentioned items.

Copy link
Contributor

Choose a reason for hiding this comment

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

@patricoferris I will verify how much of the original code I re-used and apply your review comments to the other branch as well. Thanks!


$ ./driver.exe -unused-type-warnings=true -impl unused_types.ml
type t = int
and u = string[@@deriving zero_do_warn]
include
struct
[@@@ocaml.warning "-60"]
module Zero = struct type t =
| T0 end
let zero = Zero.T0
let _ = zero
end[@@ocaml.doc "@inline"][@@merlin.hide ]

As you can see, turning on the flag disabled that behaviour as expected.
Loading