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

Conversation

NathanReb
Copy link
Collaborator

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 based on top of #492 to ease testing for the feature, only the last commit is relevant.

CHANGES.md Outdated Show resolved Hide resolved
@NathanReb NathanReb force-pushed the disable-deriving-warn-34-silencing branch from c5ad841 to e06ff1f Compare May 29, 2024 09:19
@NathanReb
Copy link
Collaborator Author

CC @mbarbin, if you want to test this feature.

CC @ceastlund, I'd be curious to know if you get any warnings when enabling this flag for JS' codebase.

@mbarbin
Copy link
Contributor

mbarbin commented May 30, 2024

I created a preview package that includes this change, and tested it in a PR in a project that could benefits from that. I was able to remove my hack and coverage stayed at 100%. 👍

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 ...]`

Signed-off-by: Nathan Rebours <[email protected]>
@NathanReb NathanReb force-pushed the disable-deriving-warn-34-silencing branch from e06ff1f to c5ed5c6 Compare June 5, 2024 09:33
mbarbin added a commit to mbarbin/ppxlib that referenced this pull request Jul 15, 2024
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]>
--------------------------------------------------------------------------------

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

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


We have a flag that disable 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!

@mbarbin
Copy link
Contributor

mbarbin commented Jul 16, 2024

@patricoferris We were discussing with @NathanReb and I've made an attempt to implement this feature on top of another PR, see #511 . I am not sure which matches best what Nathan had in mind, just giving you the heads up.

@NathanReb
Copy link
Collaborator Author

Yes this PR is a bit out of date now. Since @mbarbin started working on the right implem, I'd prefer to take his contributions! Let's close this!

@NathanReb NathanReb closed this Jul 16, 2024
mbarbin added a commit to mbarbin/ppxlib that referenced this pull request Jul 16, 2024
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]>
mbarbin added a commit to mbarbin/ppxlib that referenced this pull request Jul 16, 2024
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]>
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.

3 participants