Features best practices: changing Debug representation?

Would you consider it bad practice to have a Cargo feature that changes (only) the Debug representation of various types when activated? Normally behavior-altering features are (I think correctly) frowned on, but Debug representations are unstable by convention, so it seems like it would be reasonable to make an exception for them.

Concretely, I'm thinking about adding an optional dependency on bstr to my crate and Debug-printing byte string fields as BStr instead of [u8] only when that dependency is enabled. (The actual field types will be &[u8] in any case.)

  • feature affecting Debug repr seems okay
  • features should not affect Debug repr

0 voters

Considering you are only ever meant to print Debug text to a human, enabling such a feature flag should have no noticeable effect on a downstream program's behaviour (e.g. whether it does/doesn't compile, expectations made around inputs, etc.).

If this feature breaks a downstream user's code (e.g. failed assertions or string.find()'s) then I would say it's their problem because they deliberately broke the "debug text isn't for machine comsumption" part of Debug's contract.

The only slightly legitimate use case I can think of is snapshot testing where you'll save the Debug representation of a value to disk so you can check whether the result of an operation changes over time, and enabling this feature may cause some tests to fail.

3 Likes

However, this also feels like this should be done in a more stable format, e.g. JSON, not relying on Debug.

My only slight issue with repr-changing features is that features are supposed to be additive, and not altering. However, since Debug is allowed to change at any time anyway, I would say one could language-lawyer one's way through and claim that the feature "does nothing" (of importance), so it's vacuously additive.

1 Like

That was my interpretation. If the string is meant to be opaque, then any changes to what it contains aren't actually changes, and therefore additive.

Agreed. For example, insta recommends you use YAML when doing snapshots.

1 Like

I have been going back and forth in my head about this. And I can't decide. I guess would lean toward "it's okay to change debug output since it's stated as unstable".

1 Like

In the BStr case I'm dealing wish, it would be possible to achieve the same goal in a truly additive way by implementing one of the specialized std::fmt traits (Binary, Pointer, etc.) for each bytestring-carrying type behind #[cfg(feature = "bstr")]. That's a little hacky, but maybe better than the alternative?

Or you could just detect the .alternate() flag (#) as well. After all, its purpose is to change the printed representation. (And for a byte string, it doesn't need to be overloaded for indentation, like it is for structures and tuples, because a byte string is an atomic, and will presumably be printed on a single line anyway.)

Or you could just detect the .alternate() flag (#) as well.

I thought about it, but that would still be behavior-changing/non-additive, because there isn't a separate trait for overloading the "alternate" flag. The types would {:#?}-print differently depending on whether the feature was active. And if that's okay, why not just do the same thing for {:?}?

No, what I meant was that you wouldn't need to create a feature in this case. You would simply print as BStr when the # flag is present, and as a [u8] slice when it isn't. This avoids the non-additive feature, while still giving the user a choice.

Ah, but I want the bstr dependency to be optional (because it's not used for core functionality, and my crate has zero dependencies otherwise).

I think I've settled my mind on "it's okay". I took the reasoning to its extreme: "Debug output format is different for the same data at each invocation." And as long as the data is presented regardless of format, I'm "fine" with that.

There is another choice: do it non-optionally without a dependency on bstr. I've done it: regex-automata/escape.rs at 235ea69ea0045a014aed0ca9ab0471977448e3b2 · BurntSushi/regex-automata · GitHub

The only non-trivial thing in that bit of code is utf8::decode, which actually just defers to std::str::from_utf8. So it doesn't even require re-implementing UTF-8 decoding. It's not going to be the fastest thing in the world, but you probably don't need it to be fast.

Consider the Go proverb: "a little copying is better than a little dependency."

4 Likes

That's a really good point. A little copying it is!

Thanks for your great work on bstr, it's invaluable.

1 Like

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.