Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into cov-table-get
Browse files Browse the repository at this point in the history
  • Loading branch information
anthrotype committed Oct 6, 2023
2 parents 3856716 + 2e39fe2 commit f05d6ef
Show file tree
Hide file tree
Showing 13 changed files with 83 additions and 64 deletions.
27 changes: 14 additions & 13 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,20 @@ jobs:
- name: check no println! or eprintln! statements
run: resources/scripts/check_no_println.sh

clippy-lint:
name: Clippy lints
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: install stable toolchain
uses: dtolnay/rust-toolchain@stable

- name: cargo clippy --all-features
run: cargo clippy --all-features --all-targets -- -D warnings

- name: cargo clippy --no-default-features
run: cargo clippy --all-targets --no-default-features -- -D warnings
# disabled temporarily pending fixing a crash in clippy
#clippy-lint:
#name: Clippy lints
#runs-on: ubuntu-latest
#steps:
#- uses: actions/checkout@v3
#- name: install stable toolchain
#uses: dtolnay/rust-toolchain@stable

#- name: cargo clippy --all-features
#run: cargo clippy --all-features --all-targets -- -D warnings

#- name: cargo clippy --no-default-features
#run: cargo clippy --all-targets --no-default-features -- -D warnings

test-stable:
name: cargo test stable
Expand Down
3 changes: 1 addition & 2 deletions font-codegen/src/fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ impl Fields {

/// If this table has a version field, return it.
pub(crate) fn version_field(&self) -> Option<&Field> {
self.iter()
.find_map(|fld| fld.attrs.version.is_some().then_some(fld))
self.iter().find(|fld| fld.attrs.version.is_some())
}

// used for validating lengths. handles both fields and 'virtual fields',
Expand Down
91 changes: 53 additions & 38 deletions font-codegen/src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,12 +592,14 @@ fn generate_format_from_obj(
parse_module: &syn::Path,
) -> syn::Result<TokenStream> {
let name = &item.name;
let to_owned_arms = item.variants.iter().filter_map(|variant| {
variant.attrs.write_only.is_none().then(|| {
let to_owned_arms = item
.variants
.iter()
.filter(|variant| variant.attrs.write_only.is_none())
.map(|variant| {
let var_name = &variant.name;
quote!( ObjRefType::#var_name(item) => #name::#var_name(item.to_owned_table()), )
})
});
});

Ok(quote! {
impl FromObjRef<#parse_module:: #name<'_>> for #name {
Expand All @@ -623,41 +625,52 @@ fn generate_format_from_obj(
pub(crate) fn generate_format_group(item: &TableFormat) -> syn::Result<TokenStream> {
let name = &item.name;
let docs = &item.attrs.docs;
let variants = item.variants.iter().filter_map(|variant| {
variant.attrs.write_only.is_none().then(|| {
let variants = item
.variants
.iter()
.filter(|variant| variant.attrs.write_only.is_none())
.map(|variant| {
let name = &variant.name;
let typ = variant.type_name();
let docs = &variant.attrs.docs;
quote! ( #( #docs )* #name(#typ<'a>) )
})
});
});

let format = &item.format;
let match_arms = item.variants.iter().filter_map(|variant| {
if variant.attrs.write_only.is_some() {
return None;
}
let name = &variant.name;
let lhs = if let Some(expr) = variant.attrs.match_stmt.as_deref() {
let expr = &expr.expr;
quote!(format if #expr)
} else {
let typ = variant.marker_name();
quote!(#typ::FORMAT)
};
Some(quote! {
#lhs => {
Ok(Self::#name(FontRead::read(data)?))
}
// if we have any fancy match statement we disable a clippy lint
let mut has_any_match_stmt = false;
let match_arms = item
.variants
.iter()
.filter(|variant| variant.attrs.write_only.is_none())
.map(|variant| {
let name = &variant.name;
let lhs = if let Some(expr) = variant.attrs.match_stmt.as_deref() {
has_any_match_stmt = true;
let expr = &expr.expr;
quote!(format if #expr)
} else {
let typ = variant.marker_name();
quote!(#typ::FORMAT)
};
Some(quote! {
#lhs => {
Ok(Self::#name(FontRead::read(data)?))
}
})
})
});
.collect::<Vec<_>>();

let maybe_allow_lint = has_any_match_stmt.then(|| quote!(#[allow(clippy::redundant_guards)]));

let traversal_arms = item.variants.iter().filter_map(|variant| {
variant.attrs.write_only.is_none().then(|| {
let traversal_arms = item
.variants
.iter()
.filter(|variant| variant.attrs.write_only.is_none())
.map(|variant| {
let name = &variant.name;
quote!(Self::#name(table) => table)
})
});
});

let format_offset = item
.format_offset
Expand All @@ -674,6 +687,7 @@ pub(crate) fn generate_format_group(item: &TableFormat) -> syn::Result<TokenStre
impl<'a> FontRead<'a> for #name<'a> {
fn read(data: FontData<'a>) -> Result<Self, ReadError> {
let format: #format = data.read_at(#format_offset)?;
#maybe_allow_lint
match format {
#( #match_arms ),*
other => Err(ReadError::InvalidFormat(other.into())),
Expand Down Expand Up @@ -770,12 +784,12 @@ impl Table {
let mut result = Vec::new();
// if an input arg is needed later, save it in the shape.
if let Some(args) = &self.attrs.read_args {
result.extend(args.args.iter().filter_map(|arg| {
self.fields
.referenced_fields
.needs_at_runtime(&arg.ident)
.then(|| (arg.ident.clone(), arg.typ.to_token_stream()))
}));
result.extend(
args.args
.iter()
.filter(|arg| self.fields.referenced_fields.needs_at_runtime(&arg.ident))
.map(|arg| (arg.ident.clone(), arg.typ.to_token_stream())),
);
}

for next in self.fields.iter() {
Expand Down Expand Up @@ -875,15 +889,16 @@ impl TableReadArgs {
&'a self,
referenced_fields: &'a ReferencedFields,
) -> impl Iterator<Item = TokenStream> + 'a {
self.args.iter().filter_map(|TableReadArg { ident, typ }| {
referenced_fields.needs_at_runtime(ident).then(|| {
self.args
.iter()
.filter(|arg| referenced_fields.needs_at_runtime(&arg.ident))
.map(|TableReadArg { ident, typ }| {
quote! {
pub(crate) fn #ident(&self) -> #typ {
self.shape.#ident
}
}
})
})
}
}

Expand Down
2 changes: 2 additions & 0 deletions read-fonts/generated/generated_glyf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,8 @@ pub enum Glyph<'a> {
impl<'a> FontRead<'a> for Glyph<'a> {
fn read(data: FontData<'a>) -> Result<Self, ReadError> {
let format: i16 = data.read_at(0usize)?;

#[allow(clippy::redundant_guards)]
match format {
format if format >= 0 => Ok(Self::Simple(FontRead::read(data)?)),
format if format < 0 => Ok(Self::Composite(FontRead::read(data)?)),
Expand Down
2 changes: 2 additions & 0 deletions read-fonts/generated/generated_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3759,6 +3759,8 @@ pub enum DeviceOrVariationIndex<'a> {
impl<'a> FontRead<'a> for DeviceOrVariationIndex<'a> {
fn read(data: FontData<'a>) -> Result<Self, ReadError> {
let format: DeltaFormat = data.read_at(4usize)?;

#[allow(clippy::redundant_guards)]
match format {
format if format != DeltaFormat::VariationIndex => {
Ok(Self::Device(FontRead::read(data)?))
Expand Down
1 change: 0 additions & 1 deletion read-fonts/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ where
let len = self.len;

std::iter::from_fn(move || {
let args = args;
if i == len {
return None;
}
Expand Down
4 changes: 2 additions & 2 deletions read-fonts/src/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub trait Format<T> {

/// A type that can compute its size at runtime, based on some input.
///
/// For types with a constant size, see [`FixedSize`](types::FixedSize) and
/// For types with a constant size, see [`FixedSize`] and
/// for types which store their size inline, see [`VarSize`].
pub trait ComputeSize: ReadArgs {
/// Compute the number of bytes required to represent this type.
Expand All @@ -78,7 +78,7 @@ pub trait ComputeSize: ReadArgs {
///
/// As a rule, these types have an initial length field.
///
/// For types with a constant size, see [`FixedSize`](types::FixedSize) and
/// For types with a constant size, see [`FixedSize`] and
/// for types which can pre-compute their size, see [`ComputeSize`].
pub trait VarSize {
/// The type of the first (length) field of the item.
Expand Down
4 changes: 2 additions & 2 deletions read-fonts/src/tables/gsub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ include!("../../generated/generated_gsub.rs");
/// A typed GSUB [LookupList] table
pub type SubstitutionLookupList<'a> = LookupList<'a, SubstitutionLookup<'a>>;

/// A GSUB [SequenceContext](super::layout::SequenceContext)
/// A GSUB [SequenceContext]
pub type SubstitutionSequenceContext<'a> = super::layout::SequenceContext<'a>;

/// A GSUB [ChainedSequenceContext](super::layout::ChainedSequenceContext)
/// A GSUB [ChainedSequenceContext]
pub type SubstitutionChainContext<'a> = super::layout::ChainedSequenceContext<'a>;
2 changes: 1 addition & 1 deletion read-fonts/src/tables/postscript/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ fn read_offset(
//
// See <https://learn.microsoft.com/en-us/typography/opentype/spec/cff2#table-7-index-format>
if index > count {
return Err(ReadError::OutOfBounds)?;
Err(ReadError::OutOfBounds)?;
}
let data_offset = index * offset_size as usize;
let offset_data = FontData::new(offset_data);
Expand Down
4 changes: 2 additions & 2 deletions read-fonts/src/tests/gpos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ fn sequencelookuprecord() {
//FIXME: turn this back on when we support device records
//#[test]
//fn valueformattable() {
//// https://docs.microsoft.com/en-us/typography/opentype/spec/gpos#example-14-valueformat-table-and-valuerecord
// // https://docs.microsoft.com/en-us/typography/opentype/spec/gpos#example-14-valueformat-table-and-valuerecord

//#[rustfmt::skip]
//let bytes = [
Expand Down Expand Up @@ -235,7 +235,7 @@ fn anchorformat2() {
//FIXME: enable when we have device tables working
//#[test]
//fn anchorformat3() {
//// https://docs.microsoft.com/en-us/typography/opentype/spec/gpos#example-17-anchorformat3-table
// // https://docs.microsoft.com/en-us/typography/opentype/spec/gpos#example-17-anchorformat3-table

//let bytes = [
//0x00, 0x03, 0x01, 0x17, 0x05, 0x15, 0x00, 0x0A, 0x00, 0x14,
Expand Down
2 changes: 1 addition & 1 deletion write-fonts/src/tables/glyf/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ impl RepeatableFlag {
}

match (iter.next(), prev.take()) {
(None, Some(RepeatableFlag { flag, repeat })) if repeat == 1 => {
(None, Some(RepeatableFlag { flag, repeat: 1 })) => {
let flag = flag & !SimpleGlyphFlags::REPEAT_FLAG;
decompose_single_repeat = Some(RepeatableFlag { flag, repeat: 0 });
return decompose_single_repeat;
Expand Down
1 change: 1 addition & 0 deletions write-fonts/src/tables/stat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ impl Stat {
// we use a custom conversion here because we use a shim table in read-fonts
// (required because it is an offset to an array of offsets, which is too recursive for us)
// but in write-fonts we want to skip the shim table and just use a vec.
#[allow(clippy::unwrap_or_default)] // we need to be explicit to provide type info
fn convert_axis_value_offsets(
from: Result<read_fonts::tables::stat::AxisValueArray, ReadError>,
) -> OffsetMarker<Vec<OffsetMarker<AxisValue>>, WIDTH_32> {
Expand Down
4 changes: 2 additions & 2 deletions write-fonts/src/tests/test_gpos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ fn contextualposformat3() {
//FIXME: turn this back on when we support device records
//#[test]
//fn valueformattable() {
//// https://docs.microsoft.com/en-us/typography/opentype/spec/gpos#example-14-valueformat-table-and-valuerecord
// // https://docs.microsoft.com/en-us/typography/opentype/spec/gpos#example-14-valueformat-table-and-valuerecord

//#[rustfmt::skip]
//let bytes = [
Expand Down Expand Up @@ -177,7 +177,7 @@ fn anchorformat2() {
//FIXME: enable when we have device tables working
//#[test]
//fn anchorformat3() {
//// https://docs.microsoft.com/en-us/typography/opentype/spec/gpos#example-17-anchorformat3-table
// // https://docs.microsoft.com/en-us/typography/opentype/spec/gpos#example-17-anchorformat3-table

//let bytes = [
//0x00, 0x03, 0x01, 0x17, 0x05, 0x15, 0x00, 0x0A, 0x00, 0x14,
Expand Down

0 comments on commit f05d6ef

Please sign in to comment.