Skip to content

Commit

Permalink
Mark functions const where possible (#1573)
Browse files Browse the repository at this point in the history
  • Loading branch information
aaronmondal authored Feb 20, 2025
1 parent 48c12b9 commit 8b9824f
Show file tree
Hide file tree
Showing 28 changed files with 57 additions and 49 deletions.
2 changes: 1 addition & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect
build --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect

# TODO(aaronmondal): Extend these flags until we can run with clippy::pedantic.
build --@rules_rust//:clippy_flags=-Dwarnings,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new,-Dclippy::manual_let_else,-Dclippy::single_match_else,-Dclippy::redundant_closure_for_method_calls,-Dclippy::semicolon_if_nothing_returned,-Dclippy::unreadable_literal,-Dclippy::range_plus_one,-Dclippy::inconsistent_struct_constructor,-Dclippy::match_wildcard_for_single_variants,-Dclippy::implicit_clone,-Dclippy::needless_pass_by_value,-Dclippy::explicit_deref_methods,-Dclippy::trivially_copy_pass_by_ref,-Dclippy::unnecessary_wraps,-Dclippy::cast_lossless,-Dclippy::map_unwrap_or,-Dclippy::ref_as_ptr,-Dclippy::inline_always,-Dclippy::redundant_else,-Dclippy::return_self_not_must_use,-Dclippy::match_same_arms,-Dclippy::explicit_iter_loop,-Dclippy::items_after_statements,-Dclippy::explicit_into_iter_loop,-Dclippy::stable_sort_primitive,-Dclippy::ptr_as_ptr,-Dclippy::needless_raw_string_hashes,-Dclippy::default_trait_access,-Dclippy::ignored_unit_patterns,-Dclippy::needless_continue,-Dclippy::wildcard_imports,-Dclippy::doc_markdown,-Dclippy::struct_field_names,-Dclippy::implicit_hasher
build --@rules_rust//:clippy_flags=-Dwarnings,-Dclippy::missing_const_for_fn,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new,-Dclippy::manual_let_else,-Dclippy::single_match_else,-Dclippy::redundant_closure_for_method_calls,-Dclippy::semicolon_if_nothing_returned,-Dclippy::unreadable_literal,-Dclippy::range_plus_one,-Dclippy::inconsistent_struct_constructor,-Dclippy::match_wildcard_for_single_variants,-Dclippy::implicit_clone,-Dclippy::needless_pass_by_value,-Dclippy::explicit_deref_methods,-Dclippy::trivially_copy_pass_by_ref,-Dclippy::unnecessary_wraps,-Dclippy::cast_lossless,-Dclippy::map_unwrap_or,-Dclippy::ref_as_ptr,-Dclippy::inline_always,-Dclippy::redundant_else,-Dclippy::return_self_not_must_use,-Dclippy::match_same_arms,-Dclippy::explicit_iter_loop,-Dclippy::items_after_statements,-Dclippy::explicit_into_iter_loop,-Dclippy::stable_sort_primitive,-Dclippy::ptr_as_ptr,-Dclippy::needless_raw_string_hashes,-Dclippy::default_trait_access,-Dclippy::ignored_unit_patterns,-Dclippy::needless_continue,-Dclippy::wildcard_imports,-Dclippy::doc_markdown,-Dclippy::struct_field_names,-Dclippy::implicit_hasher
build --@rules_rust//:clippy.toml=//:clippy.toml

test --@rules_rust//:rustfmt.toml=//:.rustfmt.toml
Expand Down
2 changes: 1 addition & 1 deletion nativelink-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ struct NamedConfigsVisitor<T> {
}

impl<T> NamedConfigsVisitor<T> {
fn new() -> Self {
const fn new() -> Self {
NamedConfigsVisitor {
phantom: PhantomData,
}
Expand Down
1 change: 1 addition & 0 deletions nativelink-proto/gen_lib_rs_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
clippy::doc_markdown,
clippy::doc_markdown,
clippy::large_enum_variant,
clippy::missing_const_for_fn,
rustdoc::invalid_html_tags
)]
"""
Expand Down
1 change: 1 addition & 0 deletions nativelink-proto/genproto/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
clippy::doc_markdown,
clippy::doc_markdown,
clippy::large_enum_variant,
clippy::missing_const_for_fn,
rustdoc::invalid_html_tags
)]

Expand Down
20 changes: 10 additions & 10 deletions nativelink-scheduler/src/awaited_action_db/awaited_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl AwaitedAction {
}
}

pub(crate) fn version(&self) -> u64 {
pub(crate) const fn version(&self) -> u64 {
self.version.0
}

Expand All @@ -136,39 +136,39 @@ impl AwaitedAction {
self.version = AwaitedActionVersion(self.version.0 + 1);
}

pub fn action_info(&self) -> &Arc<ActionInfo> {
pub const fn action_info(&self) -> &Arc<ActionInfo> {
&self.action_info
}

pub fn operation_id(&self) -> &OperationId {
pub const fn operation_id(&self) -> &OperationId {
&self.operation_id
}

pub(crate) fn sort_key(&self) -> AwaitedActionSortKey {
pub(crate) const fn sort_key(&self) -> AwaitedActionSortKey {
self.sort_key
}

pub fn state(&self) -> &Arc<ActionState> {
pub const fn state(&self) -> &Arc<ActionState> {
&self.state
}

pub(crate) fn maybe_origin_metadata(&self) -> Option<&OriginMetadata> {
pub(crate) const fn maybe_origin_metadata(&self) -> Option<&OriginMetadata> {
self.maybe_origin_metadata.as_ref()
}

pub(crate) fn worker_id(&self) -> Option<&WorkerId> {
pub(crate) const fn worker_id(&self) -> Option<&WorkerId> {
self.worker_id.as_ref()
}

pub(crate) fn last_worker_updated_timestamp(&self) -> SystemTime {
pub(crate) const fn last_worker_updated_timestamp(&self) -> SystemTime {
self.last_worker_updated_timestamp
}

pub(crate) fn worker_keep_alive(&mut self, now: SystemTime) {
self.last_worker_updated_timestamp = now;
}

pub(crate) fn last_client_keepalive_timestamp(&self) -> SystemTime {
pub(crate) const fn last_client_keepalive_timestamp(&self) -> SystemTime {
self.last_client_keepalive_timestamp
}

Expand Down Expand Up @@ -251,7 +251,7 @@ impl AwaitedActionSortKey {
Self::new(priority, timestamp)
}

pub(crate) fn as_u64(self) -> u64 {
pub(crate) const fn as_u64(self) -> u64 {
self.0
}
}
Expand Down
7 changes: 5 additions & 2 deletions nativelink-scheduler/src/memory_awaited_action_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,17 @@ struct ClientAwaitedAction {
}

impl ClientAwaitedAction {
pub fn new(operation_id: OperationId, event_tx: mpsc::UnboundedSender<ActionEvent>) -> Self {
pub const fn new(
operation_id: OperationId,
event_tx: mpsc::UnboundedSender<ActionEvent>,
) -> Self {
Self {
operation_id,
event_tx,
}
}

pub fn operation_id(&self) -> &OperationId {
pub const fn operation_id(&self) -> &OperationId {
&self.operation_id
}
}
Expand Down
6 changes: 3 additions & 3 deletions nativelink-scheduler/src/simple_scheduler_state_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ where
I: InstantWrapper,
NowFn: Fn() -> I + Clone + Send + Unpin + Sync + 'static,
{
fn new(
const fn new(
sub: U,
simple_scheduler_state_manager: Weak<SimpleSchedulerStateManager<T, I, NowFn>>,
no_event_action_timeout: Duration,
Expand Down Expand Up @@ -135,7 +135,7 @@ where
I: InstantWrapper,
NowFn: Fn() -> I + Clone + Send + Unpin + Sync + 'static,
{
fn new(
const fn new(
awaited_action_sub: U,
simple_scheduler_state_manager: Weak<SimpleSchedulerStateManager<T, I, NowFn>>,
no_event_action_timeout: Duration,
Expand Down Expand Up @@ -635,7 +635,7 @@ where
where
F: Fn(T::Subscriber) -> Box<dyn ActionStateResult> + Send + Sync + 'a,
{
fn sorted_awaited_action_state_for_flags(
const fn sorted_awaited_action_state_for_flags(
stage: OperationStageFlags,
) -> Option<SortedAwaitedActionState> {
match stage {
Expand Down
4 changes: 2 additions & 2 deletions nativelink-scheduler/src/store_awaited_action_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ where
I: InstantWrapper,
NowFn: Fn() -> I,
{
fn new(
const fn new(
maybe_client_operation_id: Option<ClientOperationId>,
subscription_key: OperationIdToAwaitedAction<'static>,
weak_store: Weak<S>,
Expand Down Expand Up @@ -303,7 +303,7 @@ impl SchedulerStoreDecodeTo for SearchStateToAwaitedAction {
}
}

fn get_state_prefix(state: SortedAwaitedActionState) -> &'static str {
const fn get_state_prefix(state: SortedAwaitedActionState) -> &'static str {
match state {
SortedAwaitedActionState::CacheCheck => "cache_check",
SortedAwaitedActionState::Queued => "queued",
Expand Down
2 changes: 1 addition & 1 deletion nativelink-scheduler/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ impl Worker {
}
}

pub fn can_accept_work(&self) -> bool {
pub const fn can_accept_work(&self) -> bool {
!self.is_paused && !self.is_draining
}
}
Expand Down
2 changes: 1 addition & 1 deletion nativelink-scheduler/tests/utils/scheduler_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl TokioWatchActionStateResult {
// Note: This function is only used in tests, but for some reason
// rust doesn't detect it as used.
#[allow(dead_code)]
pub fn new(
pub const fn new(
client_operation_id: OperationId,
action_info: Arc<ActionInfo>,
rx: watch::Receiver<Arc<ActionState>>,
Expand Down
2 changes: 1 addition & 1 deletion nativelink-service/src/execution_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ struct NativelinkOperationId {
}

impl NativelinkOperationId {
fn new(instance_name: InstanceInfoName, client_operation_id: OperationId) -> Self {
const fn new(instance_name: InstanceInfoName, client_operation_id: OperationId) -> Self {
Self {
instance_name,
client_operation_id,
Expand Down
2 changes: 1 addition & 1 deletion nativelink-service/src/health_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub struct HealthServer {
}

impl HealthServer {
pub fn new(health_registry: HealthRegistry) -> Self {
pub const fn new(health_registry: HealthRegistry) -> Self {
Self { health_registry }
}
}
Expand Down
2 changes: 1 addition & 1 deletion nativelink-service/tests/worker_api_server_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ struct TestContext {
}

#[allow(clippy::unnecessary_wraps)]
fn static_now_fn() -> Result<Duration, Error> {
const fn static_now_fn() -> Result<Duration, Error> {
Ok(Duration::from_secs(BASE_NOW_S))
}

Expand Down
2 changes: 1 addition & 1 deletion nativelink-store/src/compression_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ pub struct Footer {
/// `lz4_flex::block::get_maximum_output_size()` way over estimates, so we use the
/// one provided here: <https://github.com/torvalds/linux/blob/master/include/linux/lz4.h#L61>
/// Local testing shows this gives quite accurate worst case given random input.
fn lz4_compress_bound(input_size: u64) -> u64 {
const fn lz4_compress_bound(input_size: u64) -> u64 {
input_size + (input_size / 255) + 16
}

Expand Down
4 changes: 2 additions & 2 deletions nativelink-store/src/fast_slow_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ impl FastSlowStore {
})
}

pub fn fast_store(&self) -> &Store {
pub const fn fast_store(&self) -> &Store {
&self.fast_store
}

pub fn slow_store(&self) -> &Store {
pub const fn slow_store(&self) -> &Store {
&self.slow_store
}

Expand Down
2 changes: 1 addition & 1 deletion nativelink-util/src/buf_channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ impl DropCloserReadHalf {
}

/// The number of bytes received over this stream so far.
pub fn get_bytes_received(&self) -> u64 {
pub const fn get_bytes_received(&self) -> u64 {
self.bytes_received
}

Expand Down
2 changes: 1 addition & 1 deletion nativelink-util/src/chunked_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ where
F: FnMut(Bound<K>, Bound<K>, VecDeque<T>) -> Fut,
Fut: Future<Output = Result<Option<((Bound<K>, Bound<K>), VecDeque<T>)>, E>>,
{
pub fn new(start_key: Bound<K>, end_key: Bound<K>, chunk_fn: F) -> Self {
pub const fn new(start_key: Bound<K>, end_key: Bound<K>, chunk_fn: F) -> Self {
Self {
chunk_fn,
buffer: VecDeque::new(),
Expand Down
2 changes: 1 addition & 1 deletion nativelink-util/src/metrics_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ struct DropCounter<'a> {

impl<'a> DropCounter<'a> {
#[inline]
pub fn new(counter: &'a AtomicU64) -> Self {
pub const fn new(counter: &'a AtomicU64) -> Self {
Self { counter }
}
}
Expand Down
2 changes: 1 addition & 1 deletion nativelink-util/src/origin_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ impl<T> ContextAwareFuture<T> {

#[must_use = "futures do nothing unless you `.await` or poll them"]
#[inline]
pub(crate) fn new(context: Option<Arc<OriginContext>>, inner: Instrumented<T>) -> Self {
pub(crate) const fn new(context: Option<Arc<OriginContext>>, inner: Instrumented<T>) -> Self {
Self {
inner: ManuallyDrop::new(inner),
context,
Expand Down
4 changes: 2 additions & 2 deletions nativelink-util/src/origin_event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ static NODE_ID: OnceLock<[u8; 6]> = OnceLock::new();
/// meaning you may use the first nibble for other
/// purposes.
#[inline]
pub fn get_id_for_event(event: &Event) -> [u8; 2] {
pub const fn get_id_for_event(event: &Event) -> [u8; 2] {
match &event.event {
None => [0x00, 0x00],
Some(event::Event::Request(req)) => match req.event {
Expand Down Expand Up @@ -169,7 +169,7 @@ pub struct OriginEventCollector {
}

impl OriginEventCollector {
pub fn new(sender: mpsc::Sender<OriginEvent>, metadata: OriginMetadata) -> Self {
pub const fn new(sender: mpsc::Sender<OriginEvent>, metadata: OriginMetadata) -> Self {
Self { sender, metadata }
}

Expand Down
2 changes: 1 addition & 1 deletion nativelink-util/src/origin_event_publisher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub struct OriginEventPublisher {
}

impl OriginEventPublisher {
pub fn new(
pub const fn new(
store: Store,
rx: mpsc::Receiver<OriginEvent>,
shutdown_tx: broadcast::Sender<ShutdownGuard>,
Expand Down
13 changes: 8 additions & 5 deletions nativelink-util/src/proto_stream_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ where
futures::future::poll_fn(|cx| Pin::new(&mut *self).poll_next(cx)).await
}

pub fn is_first_msg(&self) -> bool {
pub const fn is_first_msg(&self) -> bool {
self.first_msg.is_some()
}
}
Expand Down Expand Up @@ -152,7 +152,10 @@ pub struct FirstStream {
}

impl FirstStream {
pub fn new(first_response: Option<ReadResponse>, stream: Streaming<ReadResponse>) -> Self {
pub const fn new(
first_response: Option<ReadResponse>,
stream: Streaming<ReadResponse>,
) -> Self {
Self {
first_response: Some(first_response),
stream,
Expand Down Expand Up @@ -200,7 +203,7 @@ where
T: Stream<Item = Result<WriteRequest, E>> + Unpin + Send + 'static,
E: Into<Error> + 'static,
{
pub fn new(instance_name: String, read_stream: WriteRequestStreamWrapper<T>) -> Self {
pub const fn new(instance_name: String, read_stream: WriteRequestStreamWrapper<T>) -> Self {
Self {
instance_name,
read_stream_error: None,
Expand Down Expand Up @@ -231,7 +234,7 @@ where
}
}

pub fn can_resume(&self) -> bool {
pub const fn can_resume(&self) -> bool {
self.read_stream_error.is_none()
&& (self.cached_messages[0].is_some() || self.read_stream.is_first_msg())
}
Expand Down Expand Up @@ -261,7 +264,7 @@ where
T: Stream<Item = Result<WriteRequest, E>> + Unpin + Send + 'static,
E: Into<Error> + 'static,
{
pub fn new(shared_state: Arc<Mutex<WriteState<T, E>>>) -> Self {
pub const fn new(shared_state: Arc<Mutex<WriteState<T, E>>>) -> Self {
Self { shared_state }
}
}
Expand Down
4 changes: 2 additions & 2 deletions nativelink-util/src/retry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ struct ExponentialBackoff {
}

impl ExponentialBackoff {
fn new(base: Duration) -> Self {
const fn new(base: Duration) -> Self {
ExponentialBackoff { current: base }
}
}
Expand Down Expand Up @@ -59,7 +59,7 @@ pub struct Retrier {
config: Retry,
}

fn to_error_code(code: Code) -> ErrorCode {
const fn to_error_code(code: Code) -> ErrorCode {
match code {
Code::Cancelled => ErrorCode::Cancelled,
Code::InvalidArgument => ErrorCode::InvalidArgument,
Expand Down
2 changes: 1 addition & 1 deletion nativelink-util/src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub struct JoinHandleDropGuard<T> {
}

impl<T> JoinHandleDropGuard<T> {
pub fn new(inner: JoinHandle<T>) -> Self {
pub const fn new(inner: JoinHandle<T>) -> Self {
Self { inner }
}
}
Expand Down
Loading

0 comments on commit 8b9824f

Please sign in to comment.