From cd91a50d41e9cba068d682210c7c3ac9d5b55d31 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sat, 11 Jan 2025 10:56:13 -0800 Subject: [PATCH 1/2] polyfill: Add helper for defining error types. No functional changes. This just reduces boilerplate, especially for future changes. --- src/digest.rs | 23 ++++++------- src/polyfill.rs | 3 ++ src/polyfill/cold_error.rs | 66 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 14 deletions(-) create mode 100644 src/polyfill/cold_error.rs diff --git a/src/digest.rs b/src/digest.rs index 9fa8ade625..15cd9ada0c 100644 --- a/src/digest.rs +++ b/src/digest.rs @@ -29,6 +29,8 @@ use crate::{ }; use core::num::Wrapping; +pub(crate) use self::finish_error::FinishError; + mod dynstate; mod sha1; mod sha2; @@ -146,27 +148,20 @@ impl BlockContext { pub(crate) type InputTooLongError = error::InputTooLongError; -pub(crate) enum FinishError { - #[allow(dead_code)] - InputTooLong(InputTooLongError), - #[allow(dead_code)] - PendingNotAPartialBlock(usize), +cold_exhaustive_error! { + enum finish_error::FinishError { + input_too_long => InputTooLong(InputTooLongError), + pending_not_a_partial_block_inner => PendingNotAPartialBlock(usize), + } } impl FinishError { - #[cold] - #[inline(never)] - fn input_too_long(source: InputTooLongError) -> Self { - Self::InputTooLong(source) - } - - // unreachable #[cold] #[inline(never)] fn pending_not_a_partial_block(padding: Option<&[u8]>) -> Self { match padding { - None => Self::PendingNotAPartialBlock(0), - Some(padding) => Self::PendingNotAPartialBlock(padding.len()), + None => Self::pending_not_a_partial_block_inner(0), + Some(padding) => Self::pending_not_a_partial_block_inner(padding.len()), } } } diff --git a/src/polyfill.rs b/src/polyfill.rs index 4d5a0ec1f0..794849b54c 100644 --- a/src/polyfill.rs +++ b/src/polyfill.rs @@ -44,6 +44,9 @@ pub const fn usize_from_u64_saturated(x: u64) -> usize { } } +#[macro_use] +mod cold_error; + mod array_flat_map; mod array_split_map; diff --git a/src/polyfill/cold_error.rs b/src/polyfill/cold_error.rs new file mode 100644 index 0000000000..2c4d2911f7 --- /dev/null +++ b/src/polyfill/cold_error.rs @@ -0,0 +1,66 @@ +// Copyright 2024 Brian Smith. +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHORS DISCLAIM ALL WARRANTIES +// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY +// SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES +// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION +// OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN +// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + +/// Reduces boilerplate for defining error types where we want the compiler to +/// optimize for the non-error path by assuming constructing an error is +/// unlikely/cold code. +/// +/// WARNING: Every struct/variant must contain some *non-constant* value so +/// that the "invariant code" pass of the compiler doesn't recognize the +/// constructor as being "invariant code" and optimizing it away; +/// although such optimization would be nice to take advantage of, it +/// seems to lose the `#[cold]` attribute. +/// +/// Constructor functions ar marked `pub(super)` to ensure that instances can +/// only be constructed from within the enclosing module (and its submodules). +/// +/// XXX: #[inline(never)] is required to avoid the (MIR?) optimizer inlining +/// away the function call and losing the `#[cold]` attribute in the process. +/// We'd otherwise maybe prefer all constructors to be inline. +/// +/// The type is defined in its own submodule `#mod_name` to hide the +/// variant/struct constructor, ensuring instances are only constructed +/// through the generated `$constructor` functions. The constructor methods +/// work around the lack of the ability to mark an enum variant `#[cold]` and +/// `#[inline(never)]`. +macro_rules! cold_exhaustive_error { + { + enum $mod_name:ident::$Error:ident { + $( + $constructor:ident => $Variant:ident($ValueType:ty), + )+ + } + } => { + mod $mod_name { + #[allow(unused_imports)] + use super::*; // So `$ValueType` is in scope. + + pub enum $Error { + $( + $Variant(#[allow(dead_code)] $ValueType) + ),+ + } + + impl $Error { + $( + #[cold] + #[inline(never)] + pub(super) fn $constructor(value: $ValueType) -> Self { + Self::$Variant(value) + } + )+ + } + } + }; +} From c6830a0b1c6d2e77b2cc67e106c009ba57428d38 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sat, 11 Jan 2025 11:04:27 -0800 Subject: [PATCH 2/2] polyfill: Extend `cold_exhaustive_error` to struct types. --- src/aead/overlapping/array.rs | 14 +++----------- src/aead/overlapping/base.rs | 11 +++-------- src/polyfill/cold_error.rs | 23 +++++++++++++++++++++++ 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/aead/overlapping/array.rs b/src/aead/overlapping/array.rs index eb047aec01..b25e3c8de7 100644 --- a/src/aead/overlapping/array.rs +++ b/src/aead/overlapping/array.rs @@ -14,6 +14,7 @@ #![cfg_attr(not(test), allow(dead_code))] +pub use self::len_mismatch_error::LenMismatchError; use super::Overlapping; use core::array::TryFromSliceError; @@ -58,15 +59,6 @@ impl Array<'_, T, N> { } } -pub struct LenMismatchError { - #[allow(dead_code)] - len: usize, -} - -impl LenMismatchError { - #[cold] - #[inline(never)] - fn new(len: usize) -> Self { - Self { len } - } +cold_exhaustive_error! { + struct len_mismatch_error::LenMismatchError { len: usize } } diff --git a/src/aead/overlapping/base.rs b/src/aead/overlapping/base.rs index a3357f6492..0879c4d9d2 100644 --- a/src/aead/overlapping/base.rs +++ b/src/aead/overlapping/base.rs @@ -12,6 +12,7 @@ // OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN // CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +pub use self::index_error::IndexError; use super::{Array, LenMismatchError}; use core::{mem, ops::RangeFrom}; @@ -130,12 +131,6 @@ impl Overlapping<'_, T> { } } -pub struct IndexError(#[allow(dead_code)] usize); - -impl IndexError { - #[cold] - #[inline(never)] - fn new(index: usize) -> Self { - Self(index) - } +cold_exhaustive_error! { + struct index_error::IndexError { index: usize } } diff --git a/src/polyfill/cold_error.rs b/src/polyfill/cold_error.rs index 2c4d2911f7..79454f6f5b 100644 --- a/src/polyfill/cold_error.rs +++ b/src/polyfill/cold_error.rs @@ -35,6 +35,29 @@ /// work around the lack of the ability to mark an enum variant `#[cold]` and /// `#[inline(never)]`. macro_rules! cold_exhaustive_error { + // struct + { + struct $mod_name:ident::$Error:ident { + $field:ident: $ValueType:ty + } + } => { + mod $mod_name { + #[allow(unused_imports)] + use super::*; // So `$ValueType` is in scope. + + pub struct $Error { #[allow(dead_code)] $field: $ValueType } + + impl $Error { + #[cold] + #[inline(never)] + pub(super) fn new($field: $ValueType) -> Self { + Self { $field } + } + } + } + }; + + // enum { enum $mod_name:ident::$Error:ident { $(