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 (bis) #511

Merged

Conversation

mbarbin
Copy link
Contributor

@mbarbin mbarbin commented Jul 15, 2024

Add -unused-type-warnings flag to the driver

This allows disabling the generation of let _ = fun (_ : t) -> () strucutre items for each type using derivers.

The feature was meant to automatically disable warning 34 when using [@@deriving ...]

The changes are inspired by #493 but adapted for #510.

Prompted by a discussion with @NathanReb in #490

@mbarbin mbarbin changed the title Disable deriving warn 34 silencing (bis) Add an -unused-type-warnings flag to the driver (bis) Jul 15, 2024
@mbarbin mbarbin force-pushed the disable-deriving-warn-34-silencing-bis branch from 2812d3b to a4710df Compare July 16, 2024 09:37
@mbarbin mbarbin marked this pull request as ready for review July 16, 2024 09:38
@NathanReb
Copy link
Collaborator

Could you rebase this please, now that #510 has been merged?

This allows disabling the generation of `let _ = fun (_ : t) -> ()`
strucutre items for each type using derivers.

The feature was meant to automatically disable warning 34 when
using `[@@deriving ...]`

This is inspired by ocaml-ppx#493 but adapted for ocaml-ppx#510.

Signed-off-by: Mathieu Barbin <[email protected]>
- Share the [Forcable_bool.Arg] logic.
- Fixes some typos.

Signed-off-by: Mathieu Barbin <[email protected]>
@mbarbin mbarbin force-pushed the disable-deriving-warn-34-silencing-bis branch from a4710df to fc295ad Compare July 16, 2024 13:22
@mbarbin
Copy link
Contributor Author

mbarbin commented Jul 16, 2024

Could you rebase this please, now that #510 has been merged?

Done. Thanks!

Copy link
Collaborator

@NathanReb NathanReb left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@NathanReb NathanReb merged commit fe4c0e9 into ocaml-ppx:main Jul 16, 2024
6 checks passed
NathanReb added a commit to NathanReb/opam-repository that referenced this pull request Jul 22, 2024
- Fix a bug where `Code_path.main_module_name` would not properly remove
  extensions from the filename and therefore return an invalid module name.
  (ocaml-ppx/ppxlib#512, @NathanReb)

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

- Make `-unused-code-warnings` flag to the driver also controls the generation
  of warning 34 silencing structure items when using `[@@deriving ...]` on type
  declarations. (ocaml-ppx/ppxlib#510, @mbarbin, @NathanReb)

- Driver: Add `-unused-code-warnings=force` command-line flag argument. (ocaml-ppx/ppxlib#490, @mbarbin)

- new functions `Ast_builder.{e,p}list_tail` that take an extra tail
  expression/pattern argument parameter compared to `Ast_builder.{e,p}list`, so
  they can build ASTs like `a :: b :: c` instead of only `[ a; b ]`.
  (ocaml-ppx/ppxlib#498, ocaml-ppx/ppxlib#502, @v-gb, @NathanReb)

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

- Add a `special_function'` variant which directly takes a `Longident.t`
  argument to avoid the issue that `Longident.t` cover distinct syntaxic classes
  which cannot be easily parsed by a common parser (ocaml-ppx/ppxlib#496, @Octachron).

- Keep location ranges consistent when migrating `Pexp_function` nodes from 5.2+
  to older versions (ocaml-ppx/ppxlib#504, @jchavarri)

- Fix `-locations-check` behaviour so it is no longer required to pass `-check`
  as well to enable location checks. (ocaml-ppx/ppxlib#506, @NathanReb)

Signed-off-by: Nathan Rebours <[email protected]>
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
- Fix a bug where `Code_path.main_module_name` would not properly remove
  extensions from the filename and therefore return an invalid module name.
  (ocaml-ppx/ppxlib#512, @NathanReb)

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

- Make `-unused-code-warnings` flag to the driver also controls the generation
  of warning 34 silencing structure items when using `[@@deriving ...]` on type
  declarations. (ocaml-ppx/ppxlib#510, @mbarbin, @NathanReb)

- Driver: Add `-unused-code-warnings=force` command-line flag argument. (ocaml-ppx/ppxlib#490, @mbarbin)

- new functions `Ast_builder.{e,p}list_tail` that take an extra tail
  expression/pattern argument parameter compared to `Ast_builder.{e,p}list`, so
  they can build ASTs like `a :: b :: c` instead of only `[ a; b ]`.
  (ocaml-ppx/ppxlib#498, ocaml-ppx/ppxlib#502, @v-gb, @NathanReb)

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

- Add a `special_function'` variant which directly takes a `Longident.t`
  argument to avoid the issue that `Longident.t` cover distinct syntaxic classes
  which cannot be easily parsed by a common parser (ocaml-ppx/ppxlib#496, @Octachron).

- Keep location ranges consistent when migrating `Pexp_function` nodes from 5.2+
  to older versions (ocaml-ppx/ppxlib#504, @jchavarri)

- Fix `-locations-check` behaviour so it is no longer required to pass `-check`
  as well to enable location checks. (ocaml-ppx/ppxlib#506, @NathanReb)

Signed-off-by: Nathan Rebours <[email protected]>
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.

2 participants