Skip to content

non_exhaustive structures #140

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

Merged
merged 5 commits into from
Mar 28, 2021
Merged
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
6 changes: 4 additions & 2 deletions .github/bors.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ delete_merged_branches = true
required_approvals = 1
timeout_sec = 14400
status = [
"build (1.36.0)",
"build (1.40.0)",
"build (stable)",
"test"
"test",
"test-strict",
"clippy_check",
]
18 changes: 17 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
continue-on-error: ${{ matrix.experimental || false }}
strategy:
matrix:
rust: [ 1.36.0, stable ]
rust: [ 1.40.0, stable ]
include:
# Test nightly but don't fail the build.
- rust: nightly
Expand Down Expand Up @@ -44,3 +44,19 @@ jobs:
- uses: actions-rs/cargo@v1
with:
command: test

test-strict:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v2
- name: Install Rust
uses: actions-rs/toolchain@v1
with:
toolchain: stable
profile: minimal
override: true
- uses: actions-rs/cargo@v1
with:
command: test
args: --all-features
2 changes: 1 addition & 1 deletion .github/workflows/clippy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: 1.36.0
toolchain: stable
override: true
components: clippy
- uses: actions-rs/clippy-check@v1
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ This project is developed and maintained by the [Tools team][team].

## Minimum Supported Rust Version (MSRV)

This crate is guaranteed to compile on stable Rust 1.36.0 and up. It *might*
This crate is guaranteed to compile on stable Rust 1.40.0 and up. It *might*
compile with older versions but that may change in any new patch release.

## License
Expand Down
4 changes: 2 additions & 2 deletions src/elementext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ impl ElementExt for Element {
Err(e) => match e.downcast_ref() {
// if tag is empty just ignore it
Some(SVDError::EmptyTag(_, _)) => Ok(None),
_ => return Err(e),
_ => Err(e),
},
Ok(s) => Ok(Some(s.to_owned())),
Ok(s) => Ok(Some(s)),
}
} else {
Ok(None)
Expand Down
27 changes: 18 additions & 9 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@

pub use anyhow::{Context, Result};
use core::u64;
#[cfg(feature = "strict")]
use once_cell::sync::Lazy;
#[cfg(feature = "strict")]
use regex::Regex;
use xmltree::Element;

#[allow(clippy::large_enum_variant)]
#[allow(clippy::large_enum_variant, clippy::upper_case_acronyms)]
#[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)]
pub enum SVDError {
#[error("Unknown endianness `{0}`")]
Expand Down Expand Up @@ -87,6 +89,7 @@ pub enum ResetValueError {
MaskTooLarge(u64, u32),
}

#[cfg(feature = "strict")]
pub(crate) fn check_name(name: &str, tag: &str) -> Result<()> {
static PATTERN: Lazy<Regex> = Lazy::new(|| Regex::new("^[_A-Za-z0-9]*$").unwrap());
if PATTERN.is_match(name) {
Expand All @@ -98,6 +101,7 @@ pub(crate) fn check_name(name: &str, tag: &str) -> Result<()> {
}
}

#[cfg(feature = "strict")]
pub(crate) fn check_dimable_name(name: &str, tag: &str) -> Result<()> {
static PATTERN: Lazy<Regex> = Lazy::new(|| {
Regex::new("^(((%s)|(%s)[_A-Za-z]{1}[_A-Za-z0-9]*)|([_A-Za-z]{1}[_A-Za-z0-9]*(\\[%s\\])?)|([_A-Za-z]{1}[_A-Za-z0-9]*(%s)?[_A-Za-z0-9]*))$").unwrap()
Expand All @@ -117,6 +121,7 @@ pub(crate) fn check_has_placeholder(name: &str, tag: &str) -> Result<()> {
}
}

#[cfg(feature = "strict")]
pub(crate) fn check_derived_name(name: &str, tag: &str) -> Result<()> {
for x in name.split('.') {
check_dimable_name(x, tag)?
Expand All @@ -127,7 +132,7 @@ pub(crate) fn check_derived_name(name: &str, tag: &str) -> Result<()> {
pub(crate) fn check_reset_value(
size: Option<u32>,
value: Option<u64>,
mask: Option<u64>,
_mask: Option<u64>,
) -> Result<()> {
const MAX_BITS: u32 = u64::MAX.count_ones();

Expand All @@ -136,20 +141,24 @@ pub(crate) fn check_reset_value(
return Err(ResetValueError::ValueTooLarge(value, size).into());
}
}
if let (Some(size), Some(mask)) = (size, mask) {
if MAX_BITS - mask.leading_zeros() > size {
return Err(ResetValueError::MaskTooLarge(mask, size).into());
#[cfg(feature = "strict")]
{
if let (Some(size), Some(mask)) = (size, _mask) {
if MAX_BITS - mask.leading_zeros() > size {
return Err(ResetValueError::MaskTooLarge(mask, size).into());
}
}
}
if let (Some(value), Some(mask)) = (value, mask) {
if value & mask != value {
return Err(ResetValueError::MaskConflict(value, mask).into());
if let (Some(value), Some(mask)) = (value, _mask) {
if value & mask != value {
return Err(ResetValueError::MaskConflict(value, mask).into());
}
}
}

Ok(())
}

#[cfg(feature = "strict")]
#[cfg(test)]
mod tests {
use crate::error::check_reset_value;
Expand Down
8 changes: 3 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@
//! use std::fs::File;
//! use std::io::Read;
//!
//! fn main() {
//! let xml = &mut String::new();
//! File::open("STM32F30x.svd").unwrap().read_to_string(xml);
//! let xml = &mut String::new();
//! File::open("STM32F30x.svd").unwrap().read_to_string(xml);
//!
//! println!("{:?}", svd::parse(xml));
//! }
//! println!("{:?}", svd::parse(xml));
//! ```
//!
//! # References
Expand Down
2 changes: 1 addition & 1 deletion src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub trait Parse {
/// Parses an optional child element with the provided name and Parse function
/// Returns an none if the child doesn't exist, Ok(Some(e)) if parsing succeeds,
/// and Err() if parsing fails.
pub fn optional<'a, T>(n: &str, e: &'a Element) -> anyhow::Result<Option<T::Object>>
pub fn optional<T>(n: &str, e: &Element) -> anyhow::Result<Option<T::Object>>
where
T: Parse<Error = anyhow::Error>,
{
Expand Down
13 changes: 6 additions & 7 deletions src/svd/clusterinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::svd::{registercluster::RegisterCluster, registerproperties::RegisterP

#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
#[derive(Clone, Debug, PartialEq)]
#[non_exhaustive]
pub struct ClusterInfo {
/// String to identify the cluster.
/// Cluster names are required to be unique within the scope of a peripheral
Expand Down Expand Up @@ -38,10 +39,6 @@ pub struct ClusterInfo {
pub default_register_properties: RegisterProperties,

pub children: Vec<RegisterCluster>,

// Reserve the right to add more fields to this struct
#[cfg_attr(feature = "serde", serde(skip))]
_extensible: (),
}

#[derive(Clone, Debug, Default, PartialEq)]
Expand Down Expand Up @@ -99,17 +96,19 @@ impl ClusterInfoBuilder {
children: self
.children
.ok_or_else(|| BuildError::Uninitialized("children".to_string()))?,
_extensible: (),
})
.validate()
}
}

impl ClusterInfo {
#[allow(clippy::unnecessary_wraps)]
fn validate(self) -> Result<Self> {
#[cfg(feature = "strict")]
check_dimable_name(&self.name, "name")?;
if let Some(name) = self.derived_from.as_ref() {
check_derived_name(name, "derivedFrom")?;
if let Some(_name) = self.derived_from.as_ref() {
#[cfg(feature = "strict")]
check_derived_name(_name, "derivedFrom")?;
} else if self.children.is_empty() {
#[cfg(feature = "strict")]
return Err(SVDError::EmptyCluster)?;
Expand Down
7 changes: 2 additions & 5 deletions src/svd/cpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::types::Parse;

#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
#[derive(Clone, Debug, PartialEq)]
#[non_exhaustive]
pub struct Cpu {
pub name: String,

Expand All @@ -32,10 +33,6 @@ pub struct Cpu {

/// Indicate whether the processor implements a vendor-specific System Tick Timer
pub has_vendor_systick: bool,

// Reserve the right to add more fields to this struct
#[cfg_attr(feature = "serde", serde(skip))]
_extensible: (),
}

#[derive(Clone, Debug, Default, PartialEq)]
Expand Down Expand Up @@ -101,13 +98,13 @@ impl CpuBuilder {
has_vendor_systick: self
.has_vendor_systick
.ok_or_else(|| BuildError::Uninitialized("has_vendor_systick".to_string()))?,
_extensible: (),
})
.validate()
}
}

impl Cpu {
#[allow(clippy::unnecessary_wraps)]
fn validate(self) -> Result<Self> {
// TODO
Ok(self)
Expand Down
8 changes: 2 additions & 6 deletions src/svd/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::svd::{cpu::Cpu, peripheral::Peripheral, registerproperties::RegisterP

#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
#[derive(Clone, Debug, PartialEq)]
#[non_exhaustive]
pub struct Device {
/// The string identifies the device or device series. Device names are required to be unique
pub name: String,
Expand Down Expand Up @@ -53,10 +54,6 @@ pub struct Device {
pub peripherals: Vec<Peripheral>,

pub default_register_properties: RegisterProperties,

// Reserve the right to add more fields to this struct
#[cfg_attr(feature = "serde", serde(skip))]
_extensible: (),
}

#[derive(Clone, Debug, Default)]
Expand Down Expand Up @@ -124,7 +121,6 @@ impl DeviceBuilder {
.peripherals
.ok_or_else(|| BuildError::Uninitialized("peripherals".to_string()))?,
default_register_properties: self.default_register_properties,
_extensible: (),
})
.validate()
}
Expand All @@ -134,7 +130,7 @@ impl Device {
fn validate(self) -> Result<Self> {
// TODO
if self.peripherals.is_empty() {
return Err(SVDError::EmptyDevice)?;
return Err(SVDError::EmptyDevice.into());
}
Ok(self)
}
Expand Down
6 changes: 1 addition & 5 deletions src/svd/dimelement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::error::*;

#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
#[derive(Clone, Debug, PartialEq)]
#[non_exhaustive]
pub struct DimElement {
/// Defines the number of elements in an array or list
pub dim: u32,
Expand All @@ -22,10 +23,6 @@ pub struct DimElement {
#[cfg_attr(feature = "serde", serde(default))]
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
pub dim_index: Option<Vec<String>>,

// Reserve the right to add more fields to this struct
#[cfg_attr(feature = "serde", serde(skip))]
_extensible: (),
}

#[derive(Clone, Debug, Default, PartialEq)]
Expand Down Expand Up @@ -57,7 +54,6 @@ impl DimElementBuilder {
.dim_increment
.ok_or_else(|| BuildError::Uninitialized("dim_increment".to_string()))?,
dim_index: self.dim_index,
_extensible: (),
})
}
}
Expand Down
7 changes: 2 additions & 5 deletions src/svd/enumeratedvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::types::Parse;

#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
#[derive(Clone, Debug, PartialEq)]
#[non_exhaustive]
pub struct EnumeratedValue {
/// String describing the semantics of the value. Can be displayed instead of the value
pub name: String,
Expand All @@ -29,10 +30,6 @@ pub struct EnumeratedValue {
#[cfg_attr(feature = "serde", serde(default))]
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
pub is_default: Option<bool>,

// Reserve the right to add more fields to this struct
#[cfg_attr(feature = "serde", serde(skip))]
_extensible: (),
}

#[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)]
Expand Down Expand Up @@ -77,14 +74,14 @@ impl EnumeratedValueBuilder {
description: self.description,
value: self.value,
is_default: self.is_default,
_extensible: (),
})
.validate()
}
}

impl EnumeratedValue {
fn validate(self) -> Result<Self> {
#[cfg(feature = "strict")]
check_name(&self.name, "name")?;
match (&self.value, &self.is_default) {
(Some(_), None) | (None, Some(_)) => Ok(self),
Expand Down
18 changes: 9 additions & 9 deletions src/svd/enumeratedvalues.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::types::Parse;

#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
#[derive(Clone, Debug, PartialEq)]
#[non_exhaustive]
pub struct EnumeratedValues {
/// Identifier for the whole enumeration section
#[cfg_attr(feature = "serde", serde(default))]
Expand All @@ -30,10 +31,6 @@ pub struct EnumeratedValues {
pub derived_from: Option<String>,

pub values: Vec<EnumeratedValue>,

// Reserve the right to add more fields to this struct
#[cfg_attr(feature = "serde", serde(skip))]
_extensible: (),
}

#[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)]
Expand Down Expand Up @@ -73,19 +70,22 @@ impl EnumeratedValuesBuilder {
usage: self.usage,
derived_from: self.derived_from,
values: self.values.unwrap_or_default(),
_extensible: (),
})
.validate()
}
}

impl EnumeratedValues {
fn validate(self) -> Result<Self> {
if let Some(name) = self.name.as_ref() {
check_name(name, "name")?;
#[cfg(feature = "strict")]
{
if let Some(name) = self.name.as_ref() {
check_name(name, "name")?;
}
}
if let Some(dname) = self.derived_from.as_ref() {
check_derived_name(dname, "derivedFrom")?;
if let Some(_dname) = self.derived_from.as_ref() {
#[cfg(feature = "strict")]
check_derived_name(_dname, "derivedFrom")?;
Ok(self)
} else if self.values.is_empty() {
Err(EnumeratedValuesError::Empty.into())
Expand Down
Loading