Skip to content

Commit

Permalink
Merge pull request #291 from Dirbaio/x86
Browse files Browse the repository at this point in the history
Enable test-only code via feature instead of target_arch.
  • Loading branch information
japaric authored Dec 1, 2020
2 parents 04cf573 + d772f5b commit a6edbd1
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 51 deletions.
7 changes: 5 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ jobs:
- name: Check that all crates build
run: RUSTFLAGS='--deny warnings' cargo check --all
shell: bash
- name: Check that crates build with unstable-test feature
run: RUSTFLAGS='--deny warnings' cargo check --all --features unstable-test
shell: bash
- name: Run tests on Ubuntu/Windows
if: matrix.os != 'macOS-latest'
run: cargo test --workspace
run: cargo test --workspace --features unstable-test
- name: Run tests on macOS
# NOTE defmt does not build for macOS because its `cortex-m-rt` dependency doesn't
# (see https://github.com/rust-embedded/cortex-m-rt/issues/74), so we cannot use
Expand Down Expand Up @@ -97,7 +100,7 @@ jobs:
with:
mdbook-version: latest
- name: Build defmt
run: cargo build
run: cargo build --features unstable-test
# NOTE order of steps is important
- name: Run book tests
working-directory: book
Expand Down
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ version = "0.1.3"
[features]
alloc = []

# WARNING: for internal use only, not covered by semver guarantees
unstable-test = ["defmt-macros/unstable-test"]

[dependencies]
defmt-macros = { path = "macros", version = "0.1.1" }
heapless = "0.5.6"
Expand Down
2 changes: 0 additions & 2 deletions book/src/introduction.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
- Output object format must be ELF
- Custom linking (linker script) is required
- Single, global logger instance (but using multiple channels is possible)
- No x86 support. The x86 architecture is exclusively used for testing at the
moment.

## Intended use

Expand Down
4 changes: 4 additions & 0 deletions macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ version = "0.1.1"
[lib]
proc-macro = true

[features]
# WARNING: for internal use only, not covered by semver guarantees
unstable-test = []

[dependencies]
defmt-parser = { path = "../parser", features = ["unstable"], version = "0.1.0" }
quote = "1.0.7"
Expand Down
35 changes: 16 additions & 19 deletions macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,10 @@ pub fn assert_(ts: TokenStream) -> TokenStream {
Level::Error,
FormatArgs {
litstr: LitStr::new(
&format!("panicked at 'assertion failed: {}'", escape_expr(&condition)),
&format!(
"panicked at 'assertion failed: {}'",
escape_expr(&condition)
),
Span2::call_site(),
),
rest: None,
Expand Down Expand Up @@ -923,19 +926,16 @@ pub fn internp(ts: TokenStream) -> TokenStream {
let sym = symbol::Symbol::new("prim", &ls).mangle();
let section = format!(".defmt.prim.{}", sym);

quote!(match () {
#[cfg(target_arch = "x86_64")]
() => {
defmt::export::fetch_add_string_index() as u8
}
#[cfg(not(target_arch = "x86_64"))]
() => {
if cfg!(feature = "unstable-test") {
quote!({ defmt::export::fetch_add_string_index() as u8 })
} else {
quote!({
#[link_section = #section]
#[export_name = #sym]
static S: u8 = 0;
&S as *const u8 as u8
}
})
})
}
.into()
}

Expand Down Expand Up @@ -991,19 +991,16 @@ fn mksym(string: &str, tag: &str, is_log_statement: bool) -> TokenStream2 {
} else {
format_ident!("S")
};
quote!(match () {
#[cfg(target_arch = "x86_64")]
() => {
defmt::export::fetch_add_string_index()
}
#[cfg(not(target_arch = "x86_64"))]
() => {
if cfg!(feature = "unstable-test") {
quote!({ defmt::export::fetch_add_string_index() })
} else {
quote!({
#[link_section = #section]
#[export_name = #sym]
static #varname: u8 = 0;
&#varname as *const u8 as usize
}
})
})
}
}

struct Write {
Expand Down
22 changes: 11 additions & 11 deletions src/export.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{Formatter, Str};

#[cfg(target_arch = "x86_64")]
#[cfg(feature = "unstable-test")]
thread_local! {
static I: core::sync::atomic::AtomicU8 =
core::sync::atomic::AtomicU8::new(0);
Expand All @@ -11,23 +11,23 @@ thread_local! {
// NOTE we limit these values to 7-bit to avoid LEB128 encoding while writing the expected answers
// in unit tests
/// For testing purposes
#[cfg(target_arch = "x86_64")]
#[cfg(feature = "unstable-test")]
pub fn fetch_string_index() -> u8 {
I.with(|i| i.load(core::sync::atomic::Ordering::Relaxed)) & 0x7f
}

/// For testing purposes
#[cfg(target_arch = "x86_64")]
#[cfg(feature = "unstable-test")]
pub fn fetch_add_string_index() -> usize {
(I.with(|i| i.fetch_add(1, core::sync::atomic::Ordering::Relaxed)) & 0x7f) as usize
}

#[cfg(target_arch = "x86_64")]
#[cfg(feature = "unstable-test")]
pub fn acquire() -> Option<Formatter> {
None
}

#[cfg(not(target_arch = "x86_64"))]
#[cfg(not(feature = "unstable-test"))]
#[inline(never)]
pub fn acquire() -> Option<Formatter> {
extern "Rust" {
Expand All @@ -36,10 +36,10 @@ pub fn acquire() -> Option<Formatter> {
unsafe { _defmt_acquire() }
}

#[cfg(target_arch = "x86_64")]
#[cfg(feature = "unstable-test")]
pub fn release(_: Formatter) {}

#[cfg(not(target_arch = "x86_64"))]
#[cfg(not(feature = "unstable-test"))]
#[inline(never)]
pub fn release(fmt: Formatter) {
extern "Rust" {
Expand All @@ -49,12 +49,12 @@ pub fn release(fmt: Formatter) {
}

/// For testing purposes
#[cfg(target_arch = "x86_64")]
#[cfg(feature = "unstable-test")]
pub fn timestamp() -> u64 {
(T.with(|i| i.fetch_add(1, core::sync::atomic::Ordering::Relaxed)) & 0x7f) as u64
}

#[cfg(not(target_arch = "x86_64"))]
#[cfg(not(feature = "unstable-test"))]
pub fn timestamp() -> u64 {
extern "Rust" {
fn _defmt_timestamp() -> u64;
Expand Down Expand Up @@ -191,12 +191,12 @@ pub fn into_result<T: sealed::IntoResult>(x: T) -> Result<T::Ok, T::Error> {
}

/// For testing purposes
#[cfg(target_arch = "x86_64")]
#[cfg(feature = "unstable-test")]
pub fn panic() -> ! {
panic!()
}

#[cfg(not(target_arch = "x86_64"))]
#[cfg(not(feature = "unstable-test"))]
pub fn panic() -> ! {
extern "Rust" {
fn _defmt_panic() -> !;
Expand Down
2 changes: 1 addition & 1 deletion src/impls.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(target_arch = "x86_64")]
#[cfg(feature = "unstable-test")]
use crate as defmt;
use defmt_macros::internp;

Expand Down
26 changes: 13 additions & 13 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
//! with an incompatible version will result in an error. This means that you have to update both
//! the host and target side if a breaking change in defmt is released.

#![cfg_attr(not(target_arch = "x86_64"), no_std)]
#![cfg_attr(not(feature = "unstable-test"), no_std)]
#![warn(missing_docs)]

#[cfg(feature = "alloc")]
Expand Down Expand Up @@ -304,9 +304,9 @@ pub struct Str {

/// Handle to a defmt logger.
pub struct Formatter {
#[cfg(not(target_arch = "x86_64"))]
#[cfg(not(feature = "unstable-test"))]
writer: NonNull<dyn Write>,
#[cfg(target_arch = "x86_64")]
#[cfg(feature = "unstable-test")]
bytes: Vec<u8>,
bool_flags: u8, // the current group of consecutive bools
bools_left: u8, // the number of bits that we can still set in bool_flag
Expand All @@ -326,8 +326,8 @@ const MAX_NUM_BOOL_FLAGS: u8 = 8;

#[doc(hidden)]
impl Formatter {
/// Only for testing on x86_64
#[cfg(target_arch = "x86_64")]
/// Only for testing
#[cfg(feature = "unstable-test")]
pub fn new() -> Self {
Self {
bytes: vec![],
Expand All @@ -338,24 +338,24 @@ impl Formatter {
}
}

/// Only for testing on x86_64
#[cfg(target_arch = "x86_64")]
/// Only for testing
#[cfg(feature = "unstable-test")]
pub fn bytes(&self) -> &[u8] {
&self.bytes
}

#[cfg(target_arch = "x86_64")]
#[cfg(feature = "unstable-test")]
pub fn write(&mut self, bytes: &[u8]) {
self.bytes.extend_from_slice(bytes)
}

#[cfg(not(target_arch = "x86_64"))]
#[cfg(not(feature = "unstable-test"))]
pub fn write(&mut self, bytes: &[u8]) {
unsafe { self.writer.as_mut().write(bytes) }
}

/// Implementation detail
#[cfg(not(target_arch = "x86_64"))]
#[cfg(not(feature = "unstable-test"))]
pub unsafe fn from_raw(writer: NonNull<dyn Write>) -> Self {
Self {
writer,
Expand All @@ -367,7 +367,7 @@ impl Formatter {
}

/// Implementation detail
#[cfg(not(target_arch = "x86_64"))]
#[cfg(not(feature = "unstable-test"))]
pub unsafe fn into_raw(self) -> NonNull<dyn Write> {
self.writer
}
Expand Down Expand Up @@ -574,8 +574,8 @@ impl Formatter {
// these need to be in a separate module or `unreachable!` will end up calling `defmt::panic` and
// this will not compile
// (using `core::unreachable!` instead of `unreachable!` doesn't help)
#[cfg(target_arch = "x86_64")]
mod x86_64 {
#[cfg(feature = "unstable-test")]
mod test_only {
use core::ptr::NonNull;

use super::Write;
Expand Down
4 changes: 1 addition & 3 deletions tests/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// depend on `defmt` and `defmt` depends on `defmt-macros` -- the circular dependency may get in
// the way of `cargo test`

// NOTE string interning is mocked on x86 to aid testing so it does not do real interning. Instead
// NOTE string interning is mocked when testing so that it does not do real interning. Instead
// the "interner" always returns a **7-bit** `u8` value that's bumped on every interning operation.
//
// In practice, this means that the following operation:
Expand Down Expand Up @@ -35,8 +35,6 @@
// Additional notes:
//
// - the mocked index is 7 bits so its LEB128 encoding is the input byte
// - the family of `info!` macros do nothing on x86; instead use `write!` which takes a formatter
// argument

use defmt::{export::fetch_string_index, write, Format, Formatter};

Expand Down

0 comments on commit a6edbd1

Please sign in to comment.