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

core: add fmt::Debug impl to dyn Values #696

Merged
merged 1 commit into from
May 5, 2020
Merged

core: add fmt::Debug impl to dyn Values #696

merged 1 commit into from
May 5, 2020

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Apr 27, 2020

Motivation

Currently, the only way to interact with Values is to record them with
a visitor. In the most common case, where typed data is not needed,
Values are recorded with their fmt::Debug implementations — a
visitor which does not implement the record_${TYPE} traits will
automatically fall back to recording all primitive value types with
Debug. However, implementing a visitor requires a bit of boilerplate:
an entire trait implementation has to be written, and a visitor object
passed to record.

Solution

This branch hopes to simplify this common case by adding a Debug
implementation to all dyn Value trait objects. This is equivalent
to a visitor that only implements record_debug, but requiring less
boilerplate.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw added kind/feature New feature or request crate/core Related to the `tracing-core` crate labels Apr 27, 2020
@hawkw hawkw requested a review from a team April 27, 2020 22:22
@hawkw hawkw self-assigned this Apr 27, 2020
Copy link
Member

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

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

Guessing this doesn't require many docs because the expected usecase just works.

@hawkw
Copy link
Member Author

hawkw commented Apr 28, 2020

Guessing this doesn't require many docs because the expected usecase just works.

It might be nice to add examples in the doctests. But, there's kind of a lot of setup involved, and currently it's challenging to actually use the fmt::Debug impl, since the only way to access field values is with a visitor. Subsequent changes will probably add ways to index individual values, and then it will be easier to use the Debug impl.

@hawkw hawkw merged commit 361a867 into master May 5, 2020
hawkw added a commit that referenced this pull request Jul 8, 2020
Changed

- Replaced use of `inner_local_macros` with `$crate::` (#729)

Added

- `must_use` warning to guards returned by `dispatcher::set_default`
  (#686)
- `fmt::Debug` impl to `dyn Value`s (#696)
- Functions to convert between `span::Id` and `NonZeroU64` (#770)
- More obvious warnings in documentation (#769)

Fixed

- Compiler error when `tracing-core/std` feature is enabled but
  `tracing/std` is not (#760)
- Clippy warning on vtable address comparison in `callsite::Identifier`
  (#749)
- Documentation formatting issues (#715, #771)

Thanks to @bkchr, @majecty, @taiki-e, @nagisa, and @nqvz for
contributing to this release!
hawkw added a commit that referenced this pull request Jul 8, 2020
### Changed

- Replaced use of `inner_local_macros` with `$crate::` (#729)

### Added

- `must_use` warning to guards returned by `dispatcher::set_default`
  (#686)
- `fmt::Debug` impl to `dyn Value`s (#696)
- Functions to convert between `span::Id` and `NonZeroU64` (#770)
- More obvious warnings in documentation (#769)

### Fixed

- Compiler error when `tracing-core/std` feature is enabled but
  `tracing/std` is not (#760)
- Clippy warning on vtable address comparison in `callsite::Identifier`
  (#749)
- Documentation formatting issues (#715, #771)

Thanks to @bkchr, @majecty, @taiki-e, @nagisa, and @nvzqz for
contributing to this release!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/core Related to the `tracing-core` crate kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants