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.

Sign up for GitHub

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

Jump to bottom

subscriber: make PartialOrd & Ord impls more correct #995

Merged
merged 2 commits into from
Sep 28, 2020
Merged

subscriber: make PartialOrd & Ord impls more correct #995

merged 2 commits into from
Sep 28, 2020

Commits on Sep 28, 2020

  1. subscriber: make PartialOrd & Ord impls more correct 

    Currently, there are some minor issues with `Ord` and `PartialOrd` impls
in `tracing_subscriber::filter::env`:

- The `Directive` and `StaticDirective` types implement `PartialOrd`
  with an implementation that never returns `None`, and then have `Ord`
  implementations that call `partial_cmp` and `expect` that the returned
  value is `Some`. This isn't necessary.
- `field::Match` implements `PartialOrd` manually but derives `Ord`.
  Since these traits must agree, using the derived implementation for
  one but manually implementing the other is potentially incorrect (see
  #991).

This branch fixes these issues. I've moved actual comparison code from
`PartialOrd` impls for `Directive` and `StaticDirective` to their `Ord`
impls, and changed `PartialOrd::partial_cmp` for those types to call
`Ord::cmp` and wrap it in a `Some`, rather than having `Ord::cmp` call
`PartialOrd::partial_cmp` and unwrap it. This way, the fact that these
comparison impls can never return `None` is encoded at the type level,
rather than having an `expect` that will always succeed.

Additionally, I've added a manual impl of `Ord` for `field::Match` and
removed the derived impl. This should make Clippy happier.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    @hawkw
    hawkw committed Sep 28, 2020
    Configuration menu
    Copy the full SHA
    2f1fb1a View commit details
    Browse the repository at this point in the history

  2. whoopsy 

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    @hawkw
    hawkw committed Sep 28, 2020
    Configuration menu
    Copy the full SHA
    65f068b View commit details
    Browse the repository at this point in the history