Fix tests on other platforms. #17
Conversation
The tests for the static metadata need to detect when they are running on an unsupported platform and pass gracefully. Add #[used] attribute to the statics so the linker doesn't remove them. Add some comments.
src/lib.rs
Outdated
| static ETW_META: $crate::_details::EventMetadata = $crate::_details::EventMetadata{ | ||
| kw: $kw, | ||
| // TODO: Hash the callsite identity at compile time, or get Identifier to implement Ord | ||
| identity: tracing_core::identify_callsite!(&CALLSITE), | ||
| event_tag: $tags as u32 | ||
| }; | ||
|
|
||
| // These two statics need to be mut to avoid compiler errors about *const EventMetadata not being Sync. | ||
| // It's unclear why mut avoids these errors. |
There was a problem hiding this comment.
I think it is because they want static const to be usable from safe code, so it must follow safe thread rules. static mut is never usable from safe code so thread rules don't have to be checked.
| @@ -6,12 +6,14 @@ use tracelogging::*; | |||
| use tracelogging_dynamic::EventBuilder; | |||
| use tracing_subscriber::registry::{LookupSpan, SpanRef}; | |||
|
|
|||
| // Items within this .rsdata section will be sorted alphabetically, thus the start is named with "0", the end "9", and each metadata "5". | |||
| // If these statics aren't mut then everything will silently fail to work. | |||
There was a problem hiding this comment.
Needs to match mutability with the variables described in the macro.
There was a problem hiding this comment.
Why though? By the time the linker gets involved, shouldn't mut/const be gone?
There was a problem hiding this comment.
mut/const affects where the object gets put into the image. mut variables must go into read/write memory. const variables can go into read-only memory.
| @@ -6,12 +6,14 @@ use tracelogging::*; | |||
| use tracelogging_dynamic::EventBuilder; | |||
| use tracing_subscriber::registry::{LookupSpan, SpanRef}; | |||
|
|
|||
| // Items within this .rsdata section will be sorted alphabetically, thus the start is named with "0", the end "9", and each metadata "5". | |||
| // If these statics aren't mut then everything will silently fail to work. | |||
There was a problem hiding this comment.
Needs to match mutability with the variables described in the macro.
src/native/mod.rs
Outdated
| @@ -41,6 +41,9 @@ pub(crate) use tracelogging_dynamic::Guid as native_guid; | |||
| #[cfg(target_os = "linux")] | |||
| pub(crate) use eventheader::Guid as native_guid; | |||
|
|
|||
| // Used to detect the noop implementation, for testing purposes | |||
| pub(crate) const MAGIC_STATICS_SENTINEL: usize = usize::from_ne_bytes(*b"NO-OPT00"); | |||
There was a problem hiding this comment.
I would expect this to fail to compile for 32-bit. Also, it is not guaranteed that this value will not occur in the wild.
There was a problem hiding this comment.
This is only used for the no-op case, which should be selected by compiler options. Can we also make the detection of no-op be detected by compiler options rather than by a sentinel? I kinda don't like the sentinel here.
There was a problem hiding this comment.
Yeah, I can do that. I was thinking less ifdefs would be better, but I don't really like the sentinel either.
src/lib.rs
Outdated
| static ETW_META: $crate::_details::EventMetadata = $crate::_details::EventMetadata{ | ||
| kw: $kw, | ||
| // TODO: Hash the callsite identity at compile time, or get Identifier to implement Ord | ||
| identity: tracing_core::identify_callsite!(&CALLSITE), | ||
| event_tag: $tags as u32 | ||
| }; | ||
|
|
||
| // These two statics need to be mut to avoid compiler errors about *const EventMetadata not being Sync. | ||
| // It's unclear why mut avoids these errors. |
There was a problem hiding this comment.
I think it is because they want static const to be usable from safe code, so it must follow safe thread rules. static mut is never usable from safe code so thread rules don't have to be checked.
The tests for the static metadata need to detect when they are running on an unsupported platform and pass gracefully.
Add #[used] attribute to the statics so the linker doesn't remove them.
Add some comments.