diff options
author | Anthony Ramine <n.oxyde@gmail.com> | 2017-02-15 17:48:36 +0100 |
---|---|---|
committer | Anthony Ramine <n.oxyde@gmail.com> | 2017-02-15 22:11:20 +0100 |
commit | a6d59d87145ad03a95c65e5f9b384b46a91687a4 (patch) | |
tree | faeb12aa1dbe9c108d963b1fb04222d16a285a49 | |
parent | 37dab8f9f2727debc015c3a16416cd80e9b235d4 (diff) | |
download | servo-a6d59d87145ad03a95c65e5f9b384b46a91687a4.tar.gz servo-a6d59d87145ad03a95c65e5f9b384b46a91687a4.zip |
Replace inheritance_integrity by trait shenanigans
For each derived DomObject impl, we also generate a dummy trait
ShouldNotImplDomObject that is implemented for all T: DomObject.
We then try to implement it for each field type except the first one.
If compilation succeed, this means that field type doesn't implement
DomObject itself otherwise it would break coherence rules.
error[E0119]: conflicting implementations of trait `dom::xmlhttprequest::_IMPL_DOMOBJECT_FOR_XMLHttpRequest::ShouldNotImplDomObject` for type `((), SomeFieldTypeThatShouldNotImplementDomObject)`:
--> /Users/nox/src/servo/components/script/dom/xmlhttprequest.rs:120:1
|
120 | #[dom_struct]
| ^^^^^^^^^^^^^
| |
| first implementation here
| conflicting implementation for `((), SomeFieldTypeThatShouldNotImplementDomObject)`
-rw-r--r-- | components/domobject_derive/lib.rs | 59 | ||||
-rw-r--r-- | components/plugins/jstraceable.rs | 3 | ||||
-rw-r--r-- | components/plugins/lib.rs | 3 | ||||
-rw-r--r-- | components/plugins/lints/inheritance_integrity.rs | 96 | ||||
-rw-r--r-- | components/plugins/lints/mod.rs | 1 | ||||
-rw-r--r-- | components/plugins/utils.rs | 30 | ||||
-rw-r--r-- | components/script/dom/bindings/reflector.rs | 3 |
7 files changed, 56 insertions, 139 deletions
diff --git a/components/domobject_derive/lib.rs b/components/domobject_derive/lib.rs index 037ae597e8b..c718b514fc5 100644 --- a/components/domobject_derive/lib.rs +++ b/components/domobject_derive/lib.rs @@ -14,17 +14,27 @@ pub fn expand_token_stream(input: proc_macro::TokenStream) -> proc_macro::TokenS fn expand_string(input: &str) -> String { let type_ = syn::parse_macro_input(input).unwrap(); - let first_field_name = if let syn::Body::Struct(syn::VariantData::Struct(ref fields)) = type_.body { - let first_field = fields.first().expect("#[derive(DomObject)] should not be applied on empty structs"); - first_field.ident.as_ref().unwrap() + let fields = if let syn::Body::Struct(syn::VariantData::Struct(ref fields)) = type_.body { + fields } else { panic!("#[derive(DomObject)] should only be applied on proper structs") }; + let (first_field, fields) = fields + .split_first() + .expect("#[derive(DomObject)] should not be applied on empty structs"); + let first_field_name = first_field.ident.as_ref().unwrap(); + let mut field_types = vec![]; + for field in fields { + if !field_types.contains(&&field.ty) { + field_types.push(&field.ty); + } + } + let name = &type_.ident; let (impl_generics, ty_generics, where_clause) = type_.generics.split_for_impl(); - let tokens = quote! { + let mut items = quote! { impl #impl_generics ::js::conversions::ToJSValConvertible for #name #ty_generics #where_clause { #[allow(unsafe_code)] unsafe fn to_jsval(&self, @@ -49,5 +59,46 @@ fn expand_string(input: &str) -> String { } }; + let mut params = quote::Tokens::new(); + params.append_separated(type_.generics.ty_params.iter().map(|param| ¶m.ident), ", "); + + // For each field in the struct, we implement ShouldNotImplDomObject for a + // pair of all the type parameters of the DomObject and and the field type. + // This allows us to support parameterized DOM objects + // such as IteratorIterable<T>. + items.append_all(field_types.iter().map(|ty| { + quote! { + impl #impl_generics ShouldNotImplDomObject for ((#params), #ty) #where_clause {} + } + })); + + let bound = syn::TyParamBound::Trait( + syn::PolyTraitRef { + bound_lifetimes: vec![], + trait_ref: syn::parse_path("::dom::bindings::reflector::DomObject").unwrap(), + }, + syn::TraitBoundModifier::None + ); + + let mut generics = type_.generics.clone(); + generics.ty_params.push(syn::TyParam { + attrs: vec![], + ident: "__T".into(), + bounds: vec![bound], + default: None, + }); + let (impl_generics, _, where_clause) = generics.split_for_impl(); + + items.append(quote! { + trait ShouldNotImplDomObject {} + impl #impl_generics ShouldNotImplDomObject for ((#params), __T) #where_clause {} + }.as_str()); + + let dummy_const = syn::Ident::new(format!("_IMPL_DOMOBJECT_FOR_{}", name)); + let tokens = quote! { + #[allow(non_upper_case_globals)] + const #dummy_const: () = { #items }; + }; + tokens.to_string() } diff --git a/components/plugins/jstraceable.rs b/components/plugins/jstraceable.rs index 609f3e51481..071a0b54fdc 100644 --- a/components/plugins/jstraceable.rs +++ b/components/plugins/jstraceable.rs @@ -16,9 +16,6 @@ pub fn expand_dom_struct(cx: &mut ExtCtxt, sp: Span, _: &MetaItem, anno: Annotat item2.attrs.push(quote_attr!(cx, #[derive(HeapSizeOf)])); item2.attrs.push(quote_attr!(cx, #[derive(DenyPublicFields)])); item2.attrs.push(quote_attr!(cx, #[derive(DomObject)])); - // #[dom_struct] gets consumed, so this lets us keep around a residue - // Do NOT register a modifier/decorator on this attribute - item2.attrs.push(quote_attr!(cx, #[_dom_struct_marker])); Annotatable::Item(P(item2)) } else { cx.span_err(sp, "#[dom_struct] applied to something other than a struct"); diff --git a/components/plugins/lib.rs b/components/plugins/lib.rs index 7b897cb10e0..03585ae3718 100644 --- a/components/plugins/lib.rs +++ b/components/plugins/lib.rs @@ -44,12 +44,9 @@ pub fn plugin_registrar(reg: &mut Registry) { MultiModifier(box jstraceable::expand_dom_struct)); reg.register_late_lint_pass(box lints::unrooted_must_root::UnrootedPass::new()); - reg.register_late_lint_pass(box lints::inheritance_integrity::InheritancePass); reg.register_early_lint_pass(box lints::ban::BanPass); - reg.register_attribute("_dom_struct_marker".to_string(), Whitelisted); reg.register_attribute("allow_unrooted_interior".to_string(), Whitelisted); reg.register_attribute("must_root".to_string(), Whitelisted); - reg.register_attribute("servo_lang".to_string(), Whitelisted); register_clippy(reg); } diff --git a/components/plugins/lints/inheritance_integrity.rs b/components/plugins/lints/inheritance_integrity.rs deleted file mode 100644 index 28dea9aeb41..00000000000 --- a/components/plugins/lints/inheritance_integrity.rs +++ /dev/null @@ -1,96 +0,0 @@ -/* 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 http://mozilla.org/MPL/2.0/. */ - -use rustc::hir::{self, def}; -use rustc::lint::{LateContext, LintPass, LintArray, Level, LateLintPass, LintContext}; -use syntax::ast; -use utils::match_lang_ty; - -declare_lint!(INHERITANCE_INTEGRITY, Deny, - "Ensures that struct fields are properly laid out for inheritance to work"); - -/// Lint for ensuring proper layout of DOM structs -/// -/// A DOM struct must have one Reflector field or one field -/// which itself is a DOM struct (in which case it must be the first field). -pub struct InheritancePass; - -impl LintPass for InheritancePass { - fn get_lints(&self) -> LintArray { - lint_array!(INHERITANCE_INTEGRITY) - } -} - -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InheritancePass { - fn check_struct_def(&mut self, cx: &LateContext, def: &hir::VariantData, _n: ast::Name, - _gen: &hir::Generics, id: ast::NodeId) { - // Lints are run post expansion, so it's fine to use - // #[_dom_struct_marker] here without also checking for #[dom_struct] - if cx.tcx.has_attr(cx.tcx.hir.local_def_id(id), "_dom_struct_marker") { - // Find the reflector, if any - let reflector_span = def.fields().iter().enumerate() - .find(|&(ctr, f)| { - if match_lang_ty(cx, &*f.ty, "reflector") { - if ctr > 0 { - cx.span_lint(INHERITANCE_INTEGRITY, f.span, - "The Reflector should be the first field of the DOM \ - struct"); - } - return true; - } - false - }) - .map(|(_, f)| f.span); - // Find all #[dom_struct] fields - let dom_spans: Vec<_> = def.fields().iter().enumerate().filter_map(|(ctr, f)| { - if let hir::TyPath(hir::QPath::Resolved(_, ref path)) = f.ty.node { - if let def::Def::PrimTy(_) = path.def { - return None; - } - if cx.tcx.has_attr(path.def.def_id(), "_dom_struct_marker") { - // If the field is not the first, it's probably - // being misused (a) - if ctr > 0 { - cx.span_lint(INHERITANCE_INTEGRITY, f.span, - "Bare DOM structs should only be used as the first field of a \ - DOM struct. Consider using JS<T> instead."); - } - return Some(f.span) - } - } - None - }).collect(); - - // We should not have both a reflector and a dom struct field - if let Some(sp) = reflector_span { - if dom_spans.len() > 0 { - let mut db = cx.struct_span_lint(INHERITANCE_INTEGRITY, - cx.tcx.hir.expect_item(id).span, - "This DOM struct has both Reflector \ - and bare DOM struct members"); - if cx.current_level(INHERITANCE_INTEGRITY) != Level::Allow { - db.span_note(sp, "Reflector found here"); - for span in &dom_spans { - db.span_note(*span, "Bare DOM struct found here"); - } - } - } - // Nor should we have more than one dom struct field - } else if dom_spans.len() > 1 { - let mut db = cx.struct_span_lint(INHERITANCE_INTEGRITY, - cx.tcx.hir.expect_item(id).span, - "This DOM struct has multiple \ - DOM struct members, only one is allowed"); - if cx.current_level(INHERITANCE_INTEGRITY) != Level::Allow { - for span in &dom_spans { - db.span_note(*span, "Bare DOM struct found here"); - } - } - } else if dom_spans.is_empty() { - cx.span_lint(INHERITANCE_INTEGRITY, cx.tcx.hir.expect_item(id).span, - "This DOM struct has no reflector or parent DOM struct"); - } - } - } -} diff --git a/components/plugins/lints/mod.rs b/components/plugins/lints/mod.rs index 5b1debed0a2..0f94e4999e0 100644 --- a/components/plugins/lints/mod.rs +++ b/components/plugins/lints/mod.rs @@ -3,5 +3,4 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ pub mod ban; -pub mod inheritance_integrity; pub mod unrooted_must_root; diff --git a/components/plugins/utils.rs b/components/plugins/utils.rs index cbda117d75c..50ff2a959a9 100644 --- a/components/plugins/utils.rs +++ b/components/plugins/utils.rs @@ -2,18 +2,13 @@ * 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/. */ -use rustc::hir::{self, def}; use rustc::hir::def_id::DefId; use rustc::lint::{LateContext, LintContext}; use syntax::ast; -use syntax::attr::mark_used; use syntax::codemap::{ExpnFormat, Span}; use syntax::ptr::P; - /// Matches a type with a provided string, and returns its type parameters if successful -/// -/// Try not to use this for types defined in crates you own, use match_lang_ty instead (for lint passes) pub fn match_ty_unwrap<'a>(ty: &'a ast::Ty, segments: &[&str]) -> Option<&'a [P<ast::Ty>]> { match ty.node { ast::TyKind::Path(_, ast::Path { segments: ref seg, .. }) => { @@ -42,31 +37,6 @@ pub fn match_ty_unwrap<'a>(ty: &'a ast::Ty, segments: &[&str]) -> Option<&'a [P< } } -/// Checks if a type has a #[servo_lang = "str"] attribute -pub fn match_lang_ty(cx: &LateContext, ty: &hir::Ty, value: &str) -> bool { - let def = match ty.node { - hir::TyPath(hir::QPath::Resolved(_, ref path)) => path.def, - _ => return false, - }; - - if let def::Def::PrimTy(_) = def { - return false; - } - - match_lang_did(cx, def.def_id(), value) -} - -pub fn match_lang_did(cx: &LateContext, did: DefId, value: &str) -> bool { - cx.tcx.get_attrs(did).iter().any(|attr| { - if attr.check_name("servo_lang") && attr.value_str().map_or(false, |v| v == value) { - mark_used(attr); - true - } else { - false - } - }) -} - /// check if a DefId's path matches the given absolute type path /// usage e.g. with /// `match_def_path(cx, id, &["core", "option", "Option"])` diff --git a/components/script/dom/bindings/reflector.rs b/components/script/dom/bindings/reflector.rs index 40d894ccd00..896cf74a34f 100644 --- a/components/script/dom/bindings/reflector.rs +++ b/components/script/dom/bindings/reflector.rs @@ -27,9 +27,8 @@ pub fn reflect_dom_object<T, U>( /// A struct to store a reference to the reflector of a DOM object. #[allow(unrooted_must_root)] -#[must_root] -#[servo_lang = "reflector"] #[derive(HeapSizeOf)] +#[must_root] // If you're renaming or moving this field, update the path in plugins::reflector as well pub struct Reflector { #[ignore_heap_size_of = "defined and measured in rust-mozjs"] |