Review nits: Narrowly scoping unsafety, and expanding comment.

This commit is contained in:
Bobby Holley 2016-01-07 11:17:31 -08:00
parent 29987a6715
commit 3b33174163
2 changed files with 9 additions and 5 deletions

View file

@ -4,8 +4,6 @@
//! Traversals over the DOM and flow trees, running the layout computations. //! Traversals over the DOM and flow trees, running the layout computations.
#![allow(unsafe_code)]
use construct::FlowConstructor; use construct::FlowConstructor;
use context::{LayoutContext, SharedLayoutContext}; use context::{LayoutContext, SharedLayoutContext};
use flow::{PostorderFlowTraversal, PreorderFlowTraversal}; use flow::{PostorderFlowTraversal, PreorderFlowTraversal};
@ -29,6 +27,7 @@ pub struct RecalcStyleAndConstructFlows<'lc> {
impl<'lc, 'ln, N: LayoutNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleAndConstructFlows<'lc> { impl<'lc, 'ln, N: LayoutNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleAndConstructFlows<'lc> {
type SharedContext = SharedLayoutContext; type SharedContext = SharedLayoutContext;
#[allow(unsafe_code)]
fn new<'a>(shared: &'a Self::SharedContext, root: OpaqueNode) -> Self { fn new<'a>(shared: &'a Self::SharedContext, root: OpaqueNode) -> Self {
// FIXME(bholley): This transmutation from &'a to &'lc is very unfortunate, but I haven't // FIXME(bholley): This transmutation from &'a to &'lc is very unfortunate, but I haven't
// found a way to avoid it despite spending several days on it (and consulting Manishearth, // found a way to avoid it despite spending several days on it (and consulting Manishearth,
@ -48,7 +47,13 @@ impl<'lc, 'ln, N: LayoutNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleAn
// that would require higher-kinded types, which don't exist yet and probably aren't coming // that would require higher-kinded types, which don't exist yet and probably aren't coming
// for a while. // for a while.
// //
// So we transmute. :-( // So we transmute. :-( This is safe because the DomTravesalContext is stack-allocated on
// the worker thread while processing a WorkUnit, whereas the borrowed SharedContext is
// live for the entire duration of the restyle. This really could _almost_ compile: all
// we'd need to do is change the signature to to |new<'a: 'lc>|, and everything would
// work great. But we can't do that, because that would cause a mismatch with the signature
// in the trait we're implementing, and we can't mention 'lc in that trait at all for the
// reasons described above.
// //
// [1] For example, the WorkQueue type needs to be parameterized on the concrete type of // [1] For example, the WorkQueue type needs to be parameterized on the concrete type of
// DomTraversalContext::SharedContext, and the WorkQueue lifetime is similar to that of the // DomTraversalContext::SharedContext, and the WorkQueue lifetime is similar to that of the

View file

@ -2,8 +2,6 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this * License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#![allow(unsafe_code)]
use context::{LocalStyleContext, SharedStyleContext, StyleContext}; use context::{LocalStyleContext, SharedStyleContext, StyleContext};
use dom::{OpaqueNode, TNode, TRestyleDamage, UnsafeNode}; use dom::{OpaqueNode, TNode, TRestyleDamage, UnsafeNode};
use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult};
@ -148,6 +146,7 @@ pub struct RecalcStyleOnly<'lc> {
impl<'lc, 'ln, N: TNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleOnly<'lc> { impl<'lc, 'ln, N: TNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleOnly<'lc> {
type SharedContext = SharedStyleContext; type SharedContext = SharedStyleContext;
#[allow(unsafe_code)]
fn new<'a>(shared: &'a Self::SharedContext, root: OpaqueNode) -> Self { fn new<'a>(shared: &'a Self::SharedContext, root: OpaqueNode) -> Self {
// See the comment in RecalcStyleAndConstructFlows::new for an explanation of why this is // See the comment in RecalcStyleAndConstructFlows::new for an explanation of why this is
// necessary. // necessary.