style: avoid `[allow|expect](dead_code)` unconditionally at crate or module level

[BugenZhao]

trafficstars

To fix this issue, one should first remove the occurrences below, then add the attributes to the specific fields or functions.

• If the field or function is always unused, mark it expect(dead_code).
• If the field or function is unused in production but used in tests, mark it cfg_attr(not(test), expect(dead_code)).
• Avoid using allow(dead_code).

Filtered with #!?\[.*dead:

https://github.com/risingwavelabs/risingwave/blob/84e89d61f86fcfb087974927417e3171d524cc15/src/batch/src/lib.rs#L15 https://github.com/risingwavelabs/risingwave/blob/84e89d61f86fcfb087974927417e3171d524cc15/src/connector/src/lib.rs#L15 https://github.com/risingwavelabs/risingwave/blob/84e89d61f86fcfb087974927417e3171d524cc15/src/frontend/src/optimizer/delta_join_solver.rs#L69 https://github.com/risingwavelabs/risingwave/blob/84e89d61f86fcfb087974927417e3171d524cc15/src/tests/regress/src/lib.rs#L15 https://github.com/risingwavelabs/risingwave/blob/84e89d61f86fcfb087974927417e3171d524cc15/src/frontend/src/lib.rs#L54-L55 https://github.com/risingwavelabs/risingwave/blob/84e89d61f86fcfb087974927417e3171d524cc15/src/storage/src/hummock/shared_buffer/mod.rs#L16-L17

[jon-chuang]

If the field or function is unused in production but used in tests, mark it cfg_attr(not(test), expect(dead_code))

Why not just mark it #[cfg(test)]?

---

I guess unless the module is mostly dead code, we should move #[expect(dead_code)] to field and function level? Or do we forbid module-level #[expect(dead_code)] entirely? Also, we should always prefer #[expect(dead_code)] to #[allow(unused)]?

[BugenZhao]

Why not just mark it #[cfg(test)]?

Yes, it's also okay to mark #[cfg(test)]. For some cases, the code might be temporarily unused by production and will be activated after the feature is fully implemented, we may use cfg_attr(not(test), expect(dead_code)).

I guess unless the module is mostly dead code, we should move #[expect(dead_code)] to field and function level? Or do we forbid module-level #[expect(dead_code)] entirely? Also, we should always prefer #[expect(dead_code)] to #[allow(unused)]?

Agreed. BTW, #[allow(unused)] is very dangerous as it also allows unused Results and unused(unpolled) futures.

[jon-chuang]

Ok, I see allow(unused) has been forbidden by https://github.com/risingwavelabs/risingwave/pull/2956

[xxchan]

During https://github.com/risingwavelabs/risingwave/pull/5593 I'm thinking that if the lint level expect > force-warn > allow, then we can ensure expect is used instead of allow. There seems to be some discussions about such kind of things in https://github.com/rust-lang/rust/issues/54503, but I didn't check it yet. (Maybe we can wait for a stablization report for an answer 🤪