OpenSK

OpenSK copied to clipboard

Generated code (like derives) don't generate warnings when unused. They also don't end up in the final binary when unused (library size and compilation time shouldn't matter and are negligible). So there seems to be no reason to feature-gate them, which currently decreases readability and increases maintenance.

Current example snippet:

#[cfg_attr(feature = "derive_debug", derive(Debug))]

Proposed example snippet:

#[derive(Debug)]

FYI: If we are afraid of mistakenly using Result::unwrap which requires Debug, we could enable the unwrap_in_result clippy lint.

I remember an issue in Tock where this actually had impact on the produced binary (perf + size). But maybe this has either been fixed in the toolchain or I'm mixing things up.

Regarding clippy, I still have to check why clippy workflow doesn't work properly in the Github workflows. It doesn't treat warnings as errors despite the corresponding flag being set.

I remember an issue in Tock where this actually had impact on the produced binary (perf + size). But maybe this has either been fixed in the toolchain or I'm mixing things up.

Ok, that's what @kaczmarczyck said too. Maybe there used to be some bug then (maybe because our target architecture wasn't well supported at that time). But now it looks like it's fixed (at least for our crypto library and persistent storage too).

Regarding clippy, I still have to check why clippy workflow doesn't work properly in the Github workflows. It doesn't treat warnings as errors despite the corresponding flag being set.

Ah ok too bad. So we might want to wait until that's fixed then. Also note that the lint I mentioned is "allow" by default, so we would need to explicitly make it "warn" (or more).

Found the issue I mentioned: https://github.com/tock/libtock-rs/issues/84

@jmichelp this is good to close, if you don't mind not having unwrap_in_result.

Or alternatively (or in addition) a workflow to print the binary size diff as a message in PR discussions.

The workflow exists, and clippy::unwrap_in_result is too strict IMO.