jonhoo / inferno

A Rust port of FlameGraph
Other
1.68k stars 125 forks source link

`inferno 0.11.8` removed sealed `CollapsePrivate` trait from public API #264

Closed str4d closed 2 years ago

str4d commented 2 years ago

We use pprof for benchmark profiling, which depends on inferno. The most recent update caused this failure in our CI, while inferno 0.11.7 compiles fine:

❯ rustc --version
rustc 1.56.1 (59eed8a2a 2021-11-01)

❯ cargo update -p inferno --precise 0.11.7
    Updating crates.io index
    Updating inferno v0.11.8 -> v0.11.7

❯ cargo check --tests
    Checking tinytemplate v1.2.1
    Checking inferno v0.11.7
    Checking criterion v0.3.6
    Checking pprof v0.8.0
    Checking halo2_proofs v0.2.0 (/path/to/halo2/halo2_proofs)
    Checking halo2_gadgets v0.2.0 (/path/to/halo2/halo2_gadgets)
    Finished dev [unoptimized + debuginfo] target(s) in 4.35s

❯ cargo update -p inferno
    Updating crates.io index
    Updating inferno v0.11.7 -> v0.11.8

❯ cargo check --tests
    Checking inferno v0.11.8
error[E0445]: crate-private trait `CollapsePrivate` in public interface
   --> /path/to/.cargo/registry/src/github.com-1ecc6299db9ec823/inferno-0.11.8/src/collapse/mod.rs:119:1
    |
119 | / impl<T> Collapse for T
120 | | where
121 | |     T: CollapsePrivate,
122 | | {
...   |
133 | |     }
134 | | }
    | |_^ can't leak crate-private trait
    |
   ::: /path/to/.cargo/registry/src/github.com-1ecc6299db9ec823/inferno-0.11.8/src/collapse/common.rs:54:1
    |
54  |   pub(crate) trait CollapsePrivate: Send + Sized {
    |   ---------------------------------------------- `CollapsePrivate` declared as crate-private

For more information about this error, try `rustc --explain E0445`.
error: could not compile `inferno` due to previous error
str4d commented 2 years ago

Looks like this was caused by https://github.com/jonhoo/inferno/commit/b8f92605ee2897a3433dd578d9c1a7a6417bd2b0 which reduced the public API surface. Sadly blanket trait impls on public traits are themselves public, regardless of the visibility of the bounds.

The solution here (as described in https://github.com/rust-lang/rust/issues/34537 among other places) is to treat CollapsePrivate as a sealed trait, where it is public but in a private module (and thus cannot be implemented by downstream library users because it cannot be named). It looks like inferno::collapse::common is crate-private, so reverting the s/pub/pub(crate) should be sufficient to resolve this issue.