Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark functions const where possible #1573

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
18 changes: 9 additions & 9 deletions nativelink-scheduler/src/awaited_action_db/awaited_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl AwaitedAction {
}
}

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

Expand All @@ -125,35 +125,35 @@ 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 worker_id(&self) -> Option<WorkerId> {
pub(crate) const fn worker_id(&self) -> Option<WorkerId> {
self.worker_id
}

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
}
pub(crate) fn update_client_keep_alive(&mut self, now: SystemTime) {
Expand Down Expand Up @@ -235,7 +235,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 @@ -57,14 +57,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 @@ -77,7 +77,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 @@ -134,7 +134,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 @@ -630,7 +630,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 @@ -70,7 +70,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 @@ -306,7 +306,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 @@ -237,7 +237,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 @@ -58,7 +58,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 @@ -62,11 +62,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/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl ResumeableFileSlot {
}
}

pub fn new_with_take(file: Take<FileSlot>, path: PathBuf, is_write: bool) -> Self {
pub const fn new_with_take(file: Take<FileSlot>, path: PathBuf, is_write: bool) -> Self {
Self {
maybe_file_slot: MaybeFileSlot::Open(file),
path,
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 @@ -337,7 +337,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 @@ -56,7 +56,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 @@ -122,7 +122,7 @@ pub struct OriginEventCollector {
}

impl OriginEventCollector {
pub fn new(
pub const fn new(
sender: mpsc::Sender<OriginEvent>,
identity: String,
bazel_metadata: Option<RequestMetadata>,
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
Loading