Skip to content
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

use MaybeUninit instead of uninitialized #412

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
10 changes: 5 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ build = "build.rs"
discard = "1.0.3"
serde = { version = "1", optional = true }
serde_json = { version = "1", optional = true }
futures-core-preview = { version = "0.3.0-alpha.15", optional = true }
futures-channel-preview = { version = "0.3.0-alpha.15", optional = true }
futures-util-preview = { version = "0.3.0-alpha.15", optional = true }
futures-executor-preview = { version = "0.3.0-alpha.15", optional = true }
futures-core = { version = "0.3.8", optional = true }
futures-channel = { version = "0.3.8", optional = true }
futures-util = { version = "0.3.8", optional = true }
futures-executor = { version = "0.3.8", optional = true }

stdweb-derive = { version = "= 0.5.3", path = "stdweb-derive" }
stdweb-internal-macros = { version = "= 0.2.9", path = "stdweb-internal-macros" }
Expand All @@ -44,7 +44,7 @@ rustc_version = "0.2"
default = ["serde", "serde_json"]
nightly = []
web_test = []
futures-support = ["futures-core-preview", "futures-channel-preview", "futures-util-preview", "futures-executor-preview"]
futures-support = ["futures-core", "futures-channel", "futures-util", "futures-executor"]
experimental_features_which_may_break_on_minor_version_bumps = ["futures-support"]
"docs-rs" = []

Expand Down
22 changes: 9 additions & 13 deletions src/webcore/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
// TODO: verify that this works correctly with pinned futures
// TODO: use FuturesUnordered (similar to LocalPool)

use futures_core::future::{FutureObj, LocalFutureObj};
use futures_executor::enter;
use futures_core::task::{Spawn, SpawnError};
use futures_util::task::{self, ArcWake};
use futures_util::future::LocalFutureObj;
use futures_util::task::{self, ArcWake, LocalSpawn, SpawnError};
use std::future::Future;
use std::task::{Poll, Context};
use std::pin::Pin;
Expand All @@ -28,12 +27,9 @@ const INITIAL_QUEUE_CAPACITY: usize = 10;
// Iterations to wait before allowing the queue to shrink
const QUEUE_SHRINK_DELAY: usize = 10;


pub(crate) type BoxedFuture = LocalFutureObj< 'static, () >;

#[derive(Debug)]
struct TaskInner {
future: BoxedFuture,
future: LocalFutureObj<'static, ()>,
executor: EventLoopExecutor,
}

Expand All @@ -50,7 +46,7 @@ unsafe impl Send for Task {}
unsafe impl Sync for Task {}

impl Task {
fn new( executor: EventLoopExecutor, future: BoxedFuture ) -> Arc< Self > {
fn new( executor: EventLoopExecutor, future: LocalFutureObj<'static, ()> ) -> Arc< Self > {
Arc::new( Self {
is_queued: Cell::new( true ),
inner: RefCell::new( TaskInner {
Expand Down Expand Up @@ -315,21 +311,21 @@ impl EventLoopExecutor {
}

#[inline]
fn spawn_local( &self, future: BoxedFuture ) {
fn spawn_local( &self, future: LocalFutureObj<'static, ()> ) {
self.0.push_task( Task::new( self.clone(), future ) );
}
}

impl Spawn for EventLoopExecutor {
impl LocalSpawn for EventLoopExecutor {
#[inline]
fn spawn_obj( &mut self, future: FutureObj< 'static, () > ) -> Result< (), SpawnError > {
self.spawn_local( future.into() );
fn spawn_local_obj( &self, future: LocalFutureObj< 'static, () > ) -> Result< (), SpawnError > {
self.spawn_local( future );
Ok( () )
}
}


pub(crate) fn spawn_local( future: BoxedFuture ) {
pub(crate) fn spawn_local( future: LocalFutureObj<'static, ()> ) {
thread_local! {
static EVENT_LOOP: EventLoopExecutor = EventLoopExecutor::new();
}
Expand Down
4 changes: 2 additions & 2 deletions src/webcore/promise_future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,9 @@ use super::promise::{Promise, DoneHandle};
/// }
/// ```
#[inline]
pub fn spawn_local< F >( future: F ) where F: Future< Output = () > + 'static {
pub fn spawn_local< F >( future: F ) where F: Future< Output = () > + 'static{
// TODO does this need to use PinBox instead ?
let future: executor::BoxedFuture = Box::new( future ).into();
let future = Box::new( future ).into();
executor::spawn_local( future );
}

Expand Down
5 changes: 3 additions & 2 deletions src/webcore/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,9 @@ macro_rules! untagged_boilerplate {
#[inline]
fn from( untagged: $untagged_type ) -> Self {
unsafe {
let mut value: SerializedValue = mem::uninitialized();
*(&mut value as *mut SerializedValue as *mut $untagged_type) = untagged;
let mut value = mem::MaybeUninit::<SerializedValue>::uninit();
*(value.as_mut_ptr() as *mut $untagged_type) = untagged;
let mut value = value.assume_init();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assume_init should only be called after the tag has also been initialized.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this is still wrong for many cases -- the assignment of untagged might fail to completely initialize both data_1 and data_2.

And finally, if untagged is a pointer type, then this would transmute a pointer to an integer, which is not something one should do. SerializedValue seems to be intended as a type that can hold arbitrary data, so u64 and u32 are the wrong types -- those types are specifically for integers, not for arbitrary data. Use e.g. MaybeUninit<u64> for arbitrary data of size 64 bits.

value.tag = $tag;
value
}
Expand Down