Rust fix compilation for no_std targets#7338
Conversation
core2 is only used if the no_std feature is active. As of technocreatives/core2@4072d2a core2 includes std features by default. To achieve no_std compataibility this must be disabled. Fixes broken no_std builds on rustc 1.62.0-nightly.
|
@CasperN Could you take a look? |
|
Hello, I'm having trouble reproducing a build error at head. I ran in |
|
cargo test doesn't ordinarily test no_std compatibility in dependency of libraries. This is likely why it looks fine there. Ref https://blog.dbrgn.ch/2019/12/24/testing-for-no-std-compatibility/ At a conf rn, can look into this in more detail later. |
|
Thanks for the explanation. Whenever you're free, can you add the no_std smoke test to |
|
@jul-sh Any update to this? |
Haven't looked into yet, but should be able to in mid September |
|
This is blocked by bbqsrc/thiserror-core2#3 getting merged upstream, as flatbuffer depends on that crate. It needs (very minor) changes to ensure no_std compatibility. Unfortunately no news from the maintainer so far. We could proceed with using a fork until bbqsrc/thiserror-core2#3 is merged. (we did so in https://github.com/project-oak/oak). |
jul-sh
changed the title
|
@CasperN Could you approve running the workflows? Attempted the CI integration. Would be surprised if it works right away (Windows runs especially might be tricky), but would be good to be able to test it. |
|
@jul-sh thanks for the fix and test. I approved the workflows, looks like CI found something. We don't publish often but we cannot publish our library on crates.io unless all our dependencies are in crates.io too. So I'm happy to submit this (once CI passes) so long the thiserror upstream change gets merged soon-ish |
|
This crate doesn't follow the appropriate idiom for how no_std is used in production in Rust projects. The pattern is to have a In your code, you then just write This has the benefit that you can easily disable std for all crates that use this pattern with |
|
Seems to me like core2 and thiserror can be removed entirely. impl Error for InvalidFlatbuffer {
fn source(&self) -> Option<&(dyn Error + 'static)> {
match self {
InvalidFlatbuffer::MissingRequiredField { .. } => None,
InvalidFlatbuffer::InconsistentUnion { .. } => None,
InvalidFlatbuffer::Utf8Error { error: source, .. } => Some(source),
InvalidFlatbuffer::MissingNullTerminator { .. } => None,
InvalidFlatbuffer::Unaligned { .. } => None,
InvalidFlatbuffer::RangeOutOfBounds { .. } => None,
InvalidFlatbuffer::SignedOffsetOutOfBounds { .. } => None,
InvalidFlatbuffer::TooManyTables { .. } => None,
InvalidFlatbuffer::ApparentSizeTooLarge { .. } => None,
InvalidFlatbuffer::DepthLimitReached { .. } => None,
}
}
}
impl core::fmt::Display for InvalidFlatbuffer {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
match self {
InvalidFlatbuffer::MissingRequiredField {
required,
error_trace,
} => {
writeln!(f, "Missing required field `{}`.\n{}", required, error_trace)?;
}
InvalidFlatbuffer::InconsistentUnion {
field,
field_type,
error_trace,
} => {
writeln!(f,"Union exactly one of union discriminant (`{}`) and value (`{}`) are present.\n{}", field_type, field, error_trace)?;
}
InvalidFlatbuffer::Utf8Error {
error,
range,
error_trace,
} => {
writeln!(
f,
"Utf8 error for string in {:?}: {}\n{}",
range, error, error_trace
)?;
}
InvalidFlatbuffer::MissingNullTerminator { range, error_trace } => {
writeln!(
f,
"String in range [{}, {}) is missing its null terminator.\n{}",
range.start, range.end, error_trace
)?;
}
InvalidFlatbuffer::Unaligned {
position,
unaligned_type,
error_trace,
} => {
writeln!(
f,
"Type `{}` at position {} is unaligned.\n{}",
unaligned_type, position, error_trace
)?;
}
InvalidFlatbuffer::RangeOutOfBounds { range, error_trace } => {
writeln!(
f,
"Range [{}, {}) is out of bounds.\n{}",
range.start, range.end, error_trace
)?;
}
InvalidFlatbuffer::SignedOffsetOutOfBounds {
soffset,
position,
error_trace,
} => {
writeln!(
f,
"Signed offset at position {} has value {} which points out of bounds.\n{}",
position, soffset, error_trace
)?;
}
InvalidFlatbuffer::TooManyTables {} => {
writeln!(f, "Too many tables.")?;
}
InvalidFlatbuffer::ApparentSizeTooLarge {} => {
writeln!(f, "Apparent size too large.")?;
}
InvalidFlatbuffer::DepthLimitReached {} => {
writeln!(f, "Nested table depth limit reached.")?;
}
}
Ok(())
}
}It's basically what Derive(Error) creates |
|
I’m happy with removing the dependency if it’s getting in the way
…On Sep 27, 2022, 09:32 -0400, Dan Lapid ***@***.***>, wrote:
Seems to me like core2 and thiserror can be removed entirely.
They are only used in verifier.rs to derive Error for one of the error types even though the other error type is implementing format regularly
Less dependencies is always good imho and no_std will be as easy as adding the following code:
impl Error for InvalidFlatbuffer {
fn source(&self) -> Option<&(dyn Error + 'static)> {
match self {
InvalidFlatbuffer::MissingRequiredField { .. } => None,
InvalidFlatbuffer::InconsistentUnion { .. } => None,
InvalidFlatbuffer::Utf8Error { error: source, .. } => Some(source),
InvalidFlatbuffer::MissingNullTerminator { .. } => None,
InvalidFlatbuffer::Unaligned { .. } => None,
InvalidFlatbuffer::RangeOutOfBounds { .. } => None,
InvalidFlatbuffer::SignedOffsetOutOfBounds { .. } => None,
InvalidFlatbuffer::TooManyTables { .. } => None,
InvalidFlatbuffer::ApparentSizeTooLarge { .. } => None,
InvalidFlatbuffer::DepthLimitReached { .. } => None,
}
}
}
impl core::fmt::Display for InvalidFlatbuffer {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
match self {
InvalidFlatbuffer::MissingRequiredField {
required,
error_trace,
} => {
writeln!(f, "Missing required field `{}`.\n{}", required, error_trace)?;
}
InvalidFlatbuffer::InconsistentUnion {
field,
field_type,
error_trace,
} => {
writeln!(f,"Union exactly one of union discriminant (`{}`) and value (`{}`) are present.\n{}", field_type, field, error_trace)?;
}
InvalidFlatbuffer::Utf8Error {
error,
range,
error_trace,
} => {
writeln!(
f,
"Utf8 error for string in {:?}: {}\n{}",
range, error, error_trace
)?;
}
InvalidFlatbuffer::MissingNullTerminator { range, error_trace } => {
writeln!(
f,
"String in range [{}, {}) is missing its null terminator.\n{}",
range.start, range.end, error_trace
)?;
}
InvalidFlatbuffer::Unaligned {
position,
unaligned_type,
error_trace,
} => {
writeln!(
f,
"Type `{}` at position {} is unaligned.\n{}",
unaligned_type, position, error_trace
)?;
}
InvalidFlatbuffer::RangeOutOfBounds { range, error_trace } => {
writeln!(
f,
"Range [{}, {}) is out of bounds.\n{}",
range.start, range.end, error_trace
)?;
}
InvalidFlatbuffer::SignedOffsetOutOfBounds {
soffset,
position,
error_trace,
} => {
writeln!(
f,
"Signed offset at position {} has value {} which points out of bounds.\n{}",
position, soffset, error_trace
)?;
}
InvalidFlatbuffer::TooManyTables {} => {
writeln!(f, "Too many tables.")?;
}
InvalidFlatbuffer::ApparentSizeTooLarge {} => {
writeln!(f, "Apparent size too large.")?;
}
InvalidFlatbuffer::DepthLimitReached {} => {
writeln!(f, "Nested table depth limit reached.")?;
}
}
Ok(())
}
}
It's basically what Derive(Error) creates
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
I'd note it's a separate issue from how the feature flags are specified. And given the extremely small amount of code using core2, would highly recommend dropping it. |
|
Friendly ping on this issue, it's currently still not possible to use flatbuffers as no_std in practice. |
#7553 was merged, you can try again, no_std should work now. |
|
@jul-sh There are merge conflicts now. |
|
This has been superseded with #7553 |
6 participants
core2 is only used if the no_std feature is active. As of technocreatives/core2@4072d2a core2 includes std features by default. To achieve no_std compataibility this must be disabled. Fixes broken no_std builds on rustc 1.62.0-nightly.