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>

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 {

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(());

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")
     }
 

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() {
+                    match inner.kind() {
-                        !has_attr(did.did(), sym.allow_unrooted_in_rc)
+                        ty::Adt(did, _) => !has_attr(did.did(), sym.allow_unrooted_in_rc),
-                    } else {
+                        ty::Alias(
-                        true
+                            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(
+        let has_attr = |symbol| {
-            item.hir_id().expect_owner(),
+            cx.tcx.has_attrs_with_path(
-            &[sym.crown, sym.unrooted_must_root_lint, sym.must_root],
+                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(
+        let has_attr = |did, name| {
-            trait_item.hir_id().expect_owner(),
+            cx.tcx
-            &[sym.crown, sym.unrooted_must_root_lint, sym.must_root],
+                .has_attrs_with_path(did, &[sym.crown, sym.unrooted_must_root_lint, name])
-        ) {
+        };
-            return;
+
-        }
+        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 \
+                            "Mismatched use of \
-                        #[crown::unrooted_must_root_lint::must_root] \
+                             #[crown::unrooted_must_root_lint::allow_unrooted_interior] \
-                        to allow binding must_root types in associated types",
+                             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);
                     })
                 }

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() {}

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() {}

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() {}

support/crown/tests/compile-fail/trait_type_impl_must_root.rs

@@ -18,4 +18,4 @@ impl Trait for TypeHolder {
     type F = Foo;
 }
 
-fn main() {}
+fn main() {}

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;

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() {}

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() {}

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() {}