diff options
author | Josh Matthews <josh@joshmatthews.net> | 2025-01-16 15:22:40 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2025-01-16 20:22:40 +0000 |
commit | a014da590a0071a4e7cc825619a152268140560b (patch) | |
tree | d0d8cbb8f8883cd18db958357d58d05e24195cf2 | |
parent | 60dc3b26fb01ac3730475113033c270e95276a69 (diff) | |
download | servo-a014da590a0071a4e7cc825619a152268140560b.tar.gz servo-a014da590a0071a4e7cc825619a152268140560b.zip |
Support future uses of traits with associated types in rooting analysis (#34359)
* crown: Support Rc<T::Promise> and callback objects parameterized over a trait..
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
* crown: Verify that attributes match between trait associated types and impls.
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
* crown: Check type aliases as part of associated type checks.
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
* crown: Add periods to all diagnostic messages.
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
* Tidy.
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
* Fix compile-fail test expectations.
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
---------
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
-rw-r--r-- | components/script/dom/console.rs | 1 | ||||
-rw-r--r-- | components/script/dom/testns.rs | 1 | ||||
-rw-r--r-- | components/script/dom/webxr/xrhand.rs | 20 | ||||
-rw-r--r-- | support/crown/src/unrooted_must_root.rs | 159 | ||||
-rw-r--r-- | support/crown/tests/compile-fail/missing_allow_in_rc.rs | 30 | ||||
-rw-r--r-- | support/crown/tests/compile-fail/missing_unrooted_interior.rs | 17 | ||||
-rw-r--r-- | support/crown/tests/compile-fail/missing_unrooted_interior_on_trait.rs | 25 | ||||
-rw-r--r-- | support/crown/tests/compile-fail/trait_type_impl_must_root.rs | 2 | ||||
-rw-r--r-- | support/crown/tests/compile-fail/type_holder_user_must_root.rs | 1 | ||||
-rw-r--r-- | support/crown/tests/run-pass/allow_interior_trait.rs | 28 | ||||
-rw-r--r-- | support/crown/tests/run-pass/allow_trait_in_rc.rs | 29 | ||||
-rw-r--r-- | support/crown/tests/run-pass/allowed_interior.rs | 16 |
12 files changed, 296 insertions, 33 deletions
diff --git a/components/script/dom/console.rs b/components/script/dom/console.rs index ae9ac12af3f..e1bcd13c7f1 100644 --- a/components/script/dom/console.rs +++ b/components/script/dom/console.rs @@ -31,6 +31,7 @@ const MAX_LOG_DEPTH: usize = 10; const MAX_LOG_CHILDREN: usize = 15; /// <https://developer.mozilla.org/en-US/docs/Web/API/Console> +#[crown::unrooted_must_root_lint::must_root] pub(crate) struct Console; impl Console { diff --git a/components/script/dom/testns.rs b/components/script/dom/testns.rs index bd9f8a20127..96dcf37aad0 100644 --- a/components/script/dom/testns.rs +++ b/components/script/dom/testns.rs @@ -4,4 +4,5 @@ // check-tidy: no specs after this line +#[crown::unrooted_must_root_lint::must_root] pub(crate) struct TestNS(()); diff --git a/components/script/dom/webxr/xrhand.rs b/components/script/dom/webxr/xrhand.rs index a9253ccb72b..b466cf32086 100644 --- a/components/script/dom/webxr/xrhand.rs +++ b/components/script/dom/webxr/xrhand.rs @@ -3,9 +3,12 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use dom_struct::dom_struct; +use js::jsapi::JSContext; +use js::rust::MutableHandleValue; use webxr_api::{FingerJoint, Hand, Joint}; use crate::dom::bindings::codegen::Bindings::XRHandBinding::{XRHandJoint, XRHandMethods}; +use crate::dom::bindings::conversions::ToJSValConvertible; use crate::dom::bindings::iterable::Iterable; use crate::dom::bindings::reflector::{reflect_dom_object, Reflector}; use crate::dom::bindings::root::{Dom, DomRoot}; @@ -163,19 +166,30 @@ impl XRHandMethods<crate::DomTypeHolder> for XRHand { } } +/// A wrapper to work around a crown error—Root<T> has a crown annotation on it that is not present +/// on the Iterable::Value associated type. The absence is harmless in this case. +pub(crate) struct ValueWrapper(pub DomRoot<XRJointSpace>); + +impl ToJSValConvertible for ValueWrapper { + #[allow(unsafe_code)] + unsafe fn to_jsval(&self, cx: *mut JSContext, rval: MutableHandleValue) { + self.0.to_jsval(cx, rval) + } +} + impl Iterable for XRHand { type Key = XRHandJoint; - type Value = DomRoot<XRJointSpace>; + type Value = ValueWrapper; fn get_iterable_length(&self) -> u32 { JOINT_SPACE_MAP.len() as u32 } - fn get_value_at_index(&self, n: u32) -> DomRoot<XRJointSpace> { + fn get_value_at_index(&self, n: u32) -> ValueWrapper { let joint = JOINT_SPACE_MAP[n as usize].1; self.spaces .get(joint) - .map(|j| DomRoot::from_ref(&**j)) + .map(|j| ValueWrapper(DomRoot::from_ref(&**j))) .expect("Failed to get joint pose") } diff --git a/support/crown/src/unrooted_must_root.rs b/support/crown/src/unrooted_must_root.rs index fe99def5c52..e0d18cb3b7a 100644 --- a/support/crown/src/unrooted_must_root.rs +++ b/support/crown/src/unrooted_must_root.rs @@ -6,7 +6,7 @@ use rustc_hir::{self as hir, intravisit as visit, ExprKind}; use rustc_lint::{LateContext, LateLintPass, LintContext, LintPass, LintStore}; use rustc_middle::ty; use rustc_session::declare_tool_lint; -use rustc_span::def_id::LocalDefId; +use rustc_span::def_id::{DefId, LocalDefId}; use rustc_span::symbol::{sym, Symbol}; use crate::common::{in_derive_expn, match_def_path}; @@ -51,6 +51,45 @@ impl UnrootedPass { } } +/// For a given associated type for a trait implementation, checks if a given crown annotation +/// is present on that type. +fn associated_type_has_attr<'tcx>( + sym: &'_ Symbols, + cx: &LateContext<'tcx>, + ty: ty::Ty<'tcx>, + attr: Symbol, +) -> bool { + let mut walker = ty.walk(); + while let Some(generic_arg) = walker.next() { + let t = match generic_arg.unpack() { + rustc_middle::ty::GenericArgKind::Type(t) => t, + _ => { + walker.skip_current_subtree(); + continue; + }, + }; + match t.kind() { + ty::Adt(did, _substs) => { + return cx.tcx.has_attrs_with_path( + did.did(), + &[sym.crown, sym.unrooted_must_root_lint, attr], + ); + }, + ty::Alias( + ty::AliasTyKind::Projection | ty::AliasTyKind::Inherent | ty::AliasTyKind::Weak, + ty, + ) => { + return cx.tcx.has_attrs_with_path( + ty.def_id, + &[sym.crown, sym.unrooted_must_root_lint, attr], + ) + }, + _ => {}, + } + } + false +} + /// Checks if a type is unrooted or contains any owned unrooted types fn is_unrooted_ty<'tcx>( sym: &'_ Symbols, @@ -82,10 +121,15 @@ fn is_unrooted_ty<'tcx>( } else if match_def_path(cx, did.did(), &[sym.alloc, sym.rc, sym.Rc]) { // Rc<Promise> is okay let inner = substs.type_at(0); - if let ty::Adt(did, _) = inner.kind() { - !has_attr(did.did(), sym.allow_unrooted_in_rc) - } else { - true + match inner.kind() { + ty::Adt(did, _) => !has_attr(did.did(), sym.allow_unrooted_in_rc), + ty::Alias( + ty::AliasTyKind::Projection | + ty::AliasTyKind::Inherent | + ty::AliasTyKind::Weak, + ty, + ) => !has_attr(ty.def_id, sym.allow_unrooted_in_rc), + _ => true, } } else if match_def_path(cx, did.did(), &[sym::core, sym.cell, sym.Ref]) || match_def_path(cx, did.did(), &[sym::core, sym.cell, sym.RefMut]) || @@ -154,6 +198,8 @@ fn is_unrooted_ty<'tcx>( if has_attr(ty.def_id, sym.must_root) { ret = true; false + } else if has_attr(ty.def_id, sym.allow_unrooted_interior) { + false } else { true } @@ -178,10 +224,13 @@ impl<'tcx> LateLintPass<'tcx> for UnrootedPass { /// must be #[crown::unrooted_must_root_lint::must_root] themselves fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item) { let sym = &self.symbols; - if cx.tcx.has_attrs_with_path( - item.hir_id().expect_owner(), - &[sym.crown, sym.unrooted_must_root_lint, sym.must_root], - ) { + let has_attr = |symbol| { + cx.tcx.has_attrs_with_path( + item.hir_id().expect_owner(), + &[sym.crown, sym.unrooted_must_root_lint, symbol], + ) + }; + if has_attr(sym.must_root) || has_attr(sym.allow_unrooted_interior) { return; } if let hir::ItemKind::Struct(def, ..) = &item.kind { @@ -191,7 +240,7 @@ impl<'tcx> LateLintPass<'tcx> for UnrootedPass { cx.lint(UNROOTED_MUST_ROOT, |lint| { lint.primary_message( "Type must be rooted, use #[crown::unrooted_must_root_lint::must_root] \ - on the struct definition to propagate" + on the struct definition to propagate." ); lint.span(field.span); }) @@ -220,7 +269,7 @@ impl<'tcx> LateLintPass<'tcx> for UnrootedPass { lint.primary_message( "Type must be rooted, \ use #[crown::unrooted_must_root_lint::must_root] \ - on the enum definition to propagate", + on the enum definition to propagate.", ); lint.span(field.ty.span); }) @@ -243,18 +292,23 @@ impl<'tcx> LateLintPass<'tcx> for UnrootedPass { }; let sym = &self.symbols; - if cx.tcx.has_attrs_with_path( - trait_item.hir_id().expect_owner(), - &[sym.crown, sym.unrooted_must_root_lint, sym.must_root], - ) { - return; - } + let has_attr = |did, name| { + cx.tcx + .has_attrs_with_path(did, &[sym.crown, sym.unrooted_must_root_lint, name]) + }; + + let def_id: DefId = trait_item.hir_id().expect_owner().into(); + let must_root_present = has_attr(def_id, sym.must_root); + + let allow_unrooted_interior_present = has_attr(def_id, sym.allow_unrooted_interior); + + let allow_unrooted_in_rc_present = has_attr(def_id, sym.allow_unrooted_in_rc); let trait_id = cx .tcx .trait_of_item(trait_item.hir_id().expect_owner().to_def_id()) .unwrap(); - // we need to make sure that all impl do not have any must_root type binded + // we need to make sure that each impl has same crown attrs let impls = cx.tcx.trait_impls_of(trait_id); for (_ty, impl_def_ids) in impls.non_blanket_impls() { for impl_def_id in impl_def_ids { @@ -263,16 +317,57 @@ impl<'tcx> LateLintPass<'tcx> for UnrootedPass { .associated_items(impl_def_id) .find_by_name_and_kind(cx.tcx, trait_item.ident, ty::AssocKind::Type, trait_id) .unwrap(); + let mir_ty = cx.tcx.type_of(type_impl.def_id).skip_binder(); - if is_unrooted_ty(&self.symbols, cx, mir_ty, false) { + + let impl_ty_must_root = associated_type_has_attr(sym, cx, mir_ty, sym.must_root); + let impl_ty_allow_unrooted = + associated_type_has_attr(sym, cx, mir_ty, sym.allow_unrooted_interior); + let impl_ty_allow_rc = + associated_type_has_attr(sym, cx, mir_ty, sym.allow_unrooted_in_rc); + + if impl_ty_must_root != must_root_present { + if !must_root_present && impl_ty_must_root { + cx.lint(UNROOTED_MUST_ROOT, |lint| { + lint.primary_message( + "Type trait declaration must be marked with \ + #[crown::unrooted_must_root_lint::must_root] \ + to allow binding must_root types in associated types.", + ); + lint.span(trait_item.span); + }); + } else { + cx.lint(UNROOTED_MUST_ROOT, |lint| { + lint.primary_message( + "Mismatched use of \ + #[crown::unrooted_must_root_lint::must_root] \ + between associated type declaration and impl definition.", + ); + lint.span(trait_item.span); + }); + } + } + + if impl_ty_allow_unrooted != allow_unrooted_interior_present { cx.lint(UNROOTED_MUST_ROOT, |lint| { lint.primary_message( - "Type trait declaration must be marked with \ - #[crown::unrooted_must_root_lint::must_root] \ - to allow binding must_root types in associated types", + "Mismatched use of \ + #[crown::unrooted_must_root_lint::allow_unrooted_interior] \ + between associated type declaration and impl definition.", ); lint.span(trait_item.span); - }) + }); + } + + if impl_ty_allow_rc != allow_unrooted_in_rc_present { + cx.lint(UNROOTED_MUST_ROOT, |lint| { + lint.primary_message( + "Mismatched use of \ + #[crown::unrooted_must_root_lint::allow_unrooted_interior_in_rc] \ + between associated type declaration and impl definition.", + ); + lint.span(trait_item.span); + }); } } } @@ -290,7 +385,10 @@ impl<'tcx> LateLintPass<'tcx> for UnrootedPass { ) { let in_new_function = match kind { visit::FnKind::ItemFn(n, _, _) | visit::FnKind::Method(n, _) => { - n.as_str() == "new" || n.as_str().starts_with("new_") || n.as_str() == "default" + n.as_str() == "new" || + n.as_str().starts_with("new_") || + n.as_str() == "default" || + n.as_str() == "Wrap" }, visit::FnKind::Closure => return, }; @@ -299,9 +397,9 @@ impl<'tcx> LateLintPass<'tcx> for UnrootedPass { let sig = cx.tcx.type_of(def_id).skip_binder().fn_sig(cx.tcx); for (arg, ty) in decl.inputs.iter().zip(sig.inputs().skip_binder().iter()) { - if is_unrooted_ty(&self.symbols, cx, *ty, false) { + if is_unrooted_ty(&self.symbols, cx, *ty, in_new_function) { cx.lint(UNROOTED_MUST_ROOT, |lint| { - lint.primary_message("Type must be rooted"); + lint.primary_message("Type must be rooted."); lint.span(arg.span); }) } @@ -311,7 +409,7 @@ impl<'tcx> LateLintPass<'tcx> for UnrootedPass { is_unrooted_ty(&self.symbols, cx, sig.output().skip_binder(), false) { cx.lint(UNROOTED_MUST_ROOT, |lint| { - lint.primary_message("Type must be rooted"); + lint.primary_message("Type must be rooted."); lint.span(decl.output.span()); }) } @@ -342,7 +440,7 @@ impl<'a, 'tcx> visit::Visitor<'tcx> for FnDefVisitor<'a, 'tcx> { let ty = cx.typeck_results().expr_ty(subexpr); if is_unrooted_ty(self.symbols, cx, ty, in_new_function) { cx.lint(UNROOTED_MUST_ROOT, |lint| { - lint.primary_message(format!("Expression of type {:?} must be rooted", ty)); + lint.primary_message(format!("Expression of type {:?} must be rooted.", ty)); lint.span(subexpr.span); }) } @@ -383,7 +481,10 @@ impl<'a, 'tcx> visit::Visitor<'tcx> for FnDefVisitor<'a, 'tcx> { let ty = cx.typeck_results().pat_ty(pat); if is_unrooted_ty(self.symbols, cx, ty, self.in_new_function) { cx.lint(UNROOTED_MUST_ROOT, |lint| { - lint.primary_message(format!("Expression of type {:?} must be rooted", ty)); + lint.primary_message(format!( + "Expression of type {:?} must be rooted.", + ty + )); lint.span(pat.span); }) } diff --git a/support/crown/tests/compile-fail/missing_allow_in_rc.rs b/support/crown/tests/compile-fail/missing_allow_in_rc.rs new file mode 100644 index 00000000000..962ed2ad2cf --- /dev/null +++ b/support/crown/tests/compile-fail/missing_allow_in_rc.rs @@ -0,0 +1,30 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +//@rustc-env:RUSTC_BOOTSTRAP=1 + +#![allow(dead_code)] + +use std::rc::Rc; + +trait TypeHolderTrait { + #[crown::unrooted_must_root_lint::must_root] + type F; + //~^ ERROR: Mismatched use of #[crown::unrooted_must_root_lint::allow_unrooted_interior_in_rc] between associated type declaration and impl definition. [crown::unrooted_must_root] +} + +struct TypeHolder; +impl TypeHolderTrait for TypeHolder { + type F = Foo; +} + +#[crown::unrooted_must_root_lint::must_root] +#[crown::unrooted_must_root_lint::allow_unrooted_in_rc] +struct Foo; + +fn foo<T: TypeHolderTrait>() -> Rc<T::F> { + //~^ ERROR: Type must be rooted. [crown::unrooted_must_root] + unimplemented!() +} + +fn main() {} diff --git a/support/crown/tests/compile-fail/missing_unrooted_interior.rs b/support/crown/tests/compile-fail/missing_unrooted_interior.rs new file mode 100644 index 00000000000..255b1204594 --- /dev/null +++ b/support/crown/tests/compile-fail/missing_unrooted_interior.rs @@ -0,0 +1,17 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +//@rustc-env:RUSTC_BOOTSTRAP=1 + +#![allow(dead_code)] + +#[crown::unrooted_must_root_lint::must_root] +struct MustBeRooted; + +struct CanBeUnrooted { + val: MustBeRooted, + //~^ ERROR: Type must be rooted, use #[crown::unrooted_must_root_lint::must_root] on the struct definition to propagate. [crown::unrooted_must_root] +} + + +fn main() {} diff --git a/support/crown/tests/compile-fail/missing_unrooted_interior_on_trait.rs b/support/crown/tests/compile-fail/missing_unrooted_interior_on_trait.rs new file mode 100644 index 00000000000..6535f97a2bd --- /dev/null +++ b/support/crown/tests/compile-fail/missing_unrooted_interior_on_trait.rs @@ -0,0 +1,25 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +//@rustc-env:RUSTC_BOOTSTRAP=1 + +#![allow(dead_code)] + +trait TypeHolderTrait { + #[crown::unrooted_must_root_lint::must_root] + type F: Default; +} + +struct TypeHolder; +impl TypeHolderTrait for TypeHolder { + type F = Foo; +} + +#[crown::unrooted_must_root_lint::must_root] +#[derive(Default)] +struct Foo; + +struct MustBeRooted<T: TypeHolderTrait>(T::F); +//~^ ERROR: Type must be rooted, use #[crown::unrooted_must_root_lint::must_root] on the struct definition to propagate. [crown::unrooted_must_root] + +fn main() {} diff --git a/support/crown/tests/compile-fail/trait_type_impl_must_root.rs b/support/crown/tests/compile-fail/trait_type_impl_must_root.rs index 1414d6e7b28..71278979b63 100644 --- a/support/crown/tests/compile-fail/trait_type_impl_must_root.rs +++ b/support/crown/tests/compile-fail/trait_type_impl_must_root.rs @@ -18,4 +18,4 @@ impl Trait for TypeHolder { type F = Foo; } -fn main() {}
\ No newline at end of file +fn main() {} diff --git a/support/crown/tests/compile-fail/type_holder_user_must_root.rs b/support/crown/tests/compile-fail/type_holder_user_must_root.rs index c1bbacd974f..b03f2b57bb0 100644 --- a/support/crown/tests/compile-fail/type_holder_user_must_root.rs +++ b/support/crown/tests/compile-fail/type_holder_user_must_root.rs @@ -11,6 +11,7 @@ struct Bar<TH: TypeHolderTrait>(TH::F); trait TypeHolderTrait { #[crown::unrooted_must_root_lint::must_root] type F; + //~^ Mismatched use of #[crown::unrooted_must_root_lint::must_root] between associated type declaration and impl definition. [crown::unrooted_must_root] } struct TypeHolder; diff --git a/support/crown/tests/run-pass/allow_interior_trait.rs b/support/crown/tests/run-pass/allow_interior_trait.rs new file mode 100644 index 00000000000..f3c08e21112 --- /dev/null +++ b/support/crown/tests/run-pass/allow_interior_trait.rs @@ -0,0 +1,28 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +//@rustc-env:RUSTC_BOOTSTRAP=1 + +#![allow(dead_code)] + +trait TypeHolderTrait { + #[crown::unrooted_must_root_lint::must_root] + type F; +} + +struct TypeHolder; +impl TypeHolderTrait for TypeHolder { + type F = Foo; +} + + +#[crown::unrooted_must_root_lint::must_root] +struct Foo; + +#[crown::unrooted_must_root_lint::must_root] +struct MustBeRooted<T: TypeHolderTrait>(T::F); + +#[crown::unrooted_must_root_lint::allow_unrooted_interior] +struct CanBeUnrooted<T: TypeHolderTrait>(MustBeRooted<T>); + +fn main() {} diff --git a/support/crown/tests/run-pass/allow_trait_in_rc.rs b/support/crown/tests/run-pass/allow_trait_in_rc.rs new file mode 100644 index 00000000000..dfbc662d3df --- /dev/null +++ b/support/crown/tests/run-pass/allow_trait_in_rc.rs @@ -0,0 +1,29 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +//@rustc-env:RUSTC_BOOTSTRAP=1 + +#![allow(dead_code)] + +use std::rc::Rc; + +trait TypeHolderTrait { + #[crown::unrooted_must_root_lint::must_root] + #[crown::unrooted_must_root_lint::allow_unrooted_in_rc] + type F; +} + +struct TypeHolder; +impl TypeHolderTrait for TypeHolder { + type F = Foo; +} + +#[crown::unrooted_must_root_lint::must_root] +#[crown::unrooted_must_root_lint::allow_unrooted_in_rc] +struct Foo; + +fn foo<T: TypeHolderTrait>() -> Rc<T::F> { + unimplemented!() +} + +fn main() {} diff --git a/support/crown/tests/run-pass/allowed_interior.rs b/support/crown/tests/run-pass/allowed_interior.rs new file mode 100644 index 00000000000..15300aaa40f --- /dev/null +++ b/support/crown/tests/run-pass/allowed_interior.rs @@ -0,0 +1,16 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +//@rustc-env:RUSTC_BOOTSTRAP=1 + +#![allow(dead_code)] + +#[crown::unrooted_must_root_lint::must_root] +struct MustBeRooted; + +#[crown::unrooted_must_root_lint::allow_unrooted_interior] +struct CanBeUnrooted { + val: MustBeRooted, +} + +fn main() {} |