search

tikv / fail-rs

Fail points for rust
Apache License 2.0
332 stars 39 forks source link

lib: do not leak third-party macros #44

Closed lucab closed 5 years ago

lucab commented 5 years ago

This re-arranges import statements in order to avoid any possible leakage of third-party macros through this crate.

Ref: https://github.com/tikv/tikv/pull/4903#issuecomment-511387150

lucab commented 5 years ago

I have honestly no clue why current master ends up leaking both lazy_static!() and info!(), as they are not marked pub use. Similarly, I have no idea why re-arranging imports this way plugs the leak.

However, I tested this change with the tikv PR referenced above, and this one works while plain 0.3.0 does not.

Hoverbear commented 5 years ago

@nrc PTAL at this? Seems a strange behavior.

nrc commented 5 years ago

OK, so, this is surprising but not technically wrong and is due to the interaction of two import mechanisms. In this crate we are using the new way (with use), this respects module scoping and privacy. In tikv_util, they use #[macro_use] extern crate fail;, i.e., the old way. The old way does not respect scoping not privacy (you can't write pub macro_rules!). In order to smooth the transition to the new way, the compiler treats macros declared in the root of the crate as always importable in the old way (so that macro re-exports can be used by crates backwards-compatibly). However, what is happening here is that the use brings the macro into scope of the top of the crate, but the macro_use in tikv_util doesn't known about privacy and so pulls it in anyway.

The solution is to either use the old way of importing macros here, the new way in tikv_util, or do what this PR does and not import macros into the top-level scope.