Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
Exhaustively match against `FieldType` to determine if it's copyable.
  • Loading branch information
bnaecker committed Jul 8, 2024
1 parent 2c68d88 commit 35432b7
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
4 changes: 3 additions & 1 deletion oximeter/impl/src/schema/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ fn emit_target(schema: &TimeseriesSchema) -> TokenStream {
fn fields_are_copyable<'a>(
mut fields: impl Iterator<Item = &'a FieldSchema>,
) -> bool {
!fields.any(|field| field.field_type == FieldType::String)
// Do a positive match, to ensure new variants don't actually derive copy
// inappropriately. Better we clone, in that case.
fields.all(FieldSchema::is_copyable)
}

fn compute_extra_derives(
Expand Down
7 changes: 7 additions & 0 deletions oximeter/impl/src/schema/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ pub struct FieldSchema {
pub description: String,
}

impl FieldSchema {
/// Return `true` if this field is copyable.
pub const fn is_copyable(&self) -> bool {
self.field_type.is_copyable()
}
}

/// The source from which a field is derived, the target or metric.
#[derive(
Clone,
Expand Down
23 changes: 23 additions & 0 deletions oximeter/impl/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,29 @@ pub enum FieldType {
Bool,
}

impl FieldType {
/// Return `true` if a field of this type is copyable.
///
/// NOTE: This doesn't mean `self` is copyable, instead refering to a field
/// with this type.
pub const fn is_copyable(&self) -> bool {
match self {
FieldType::String => false,
FieldType::I8
| FieldType::U8
| FieldType::I16
| FieldType::U16
| FieldType::I32
| FieldType::U32
| FieldType::I64
| FieldType::U64
| FieldType::IpAddr
| FieldType::Uuid
| FieldType::Bool => true,
}
}
}

impl std::fmt::Display for FieldType {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(f, "{:?}", self)
Expand Down

0 comments on commit 35432b7

Please sign in to comment.