Auto merge of #53693 - scottmcm:marker-trait-attribute, r=nikomatsakis

Support an explicit annotation for marker traits

From the tracking issue for rust-lang/rfcs#1268:
> It seems obvious that we should make a `#[marker]` annotation. ~ #29864 (comment)

This PR allows you to put `#[marker]` on a trait, at which point:
- [x] The trait must not have any items ~~All of the trait's items must have defaults~~
- [x] Any impl of the trait must be empty (not override any items)
- [x] But impls of the trait are allowed to overlap

r? @nikomatsakis

Author: bors

src/doc/unstable-book/src/language-features/marker-trait-attr.md

@@ -0,0 +1,33 @@
+# `marker_trait_attr`
+
+The tracking issue for this feature is: [#29864]
+
+[#29864]: https://github.com/rust-lang/rust/issues/29864
+
+------------------------
+
+Normally, Rust keeps you from adding trait implementations that could
+overlap with each other, as it would be ambiguous which to use.  This
+feature, however, carves out an exception to that rule: a trait can
+opt-in to having overlapping implementations, at the cost that those
+implementations are not allowed to override anything (and thus the
+trait itself cannot have any associated items, as they're pointless
+when they'd need to do the same thing for every type anyway).
+
+```rust
+#![feature(marker_trait_attr)]
+
+use std::fmt::{Debug, Display};
+
+#[marker] trait MyMarker {}
+
+impl<T: Debug> MyMarker for T {}
+impl<T: Display> MyMarker for T {}
+
+fn foo<T: MyMarker>(t: T) -> T {
+    t
+}
+```
+
+This is expected to replace the unstable `overlapping_marker_traits`
+feature, which applied to all empty traits (without needing an opt-in).

src/librustc/hir/check_attr.rs

@@ -32,6 +32,7 @@ enum Target {
     Statement,
     Closure,
     Static,
+    Trait,
     Other,
 }
 
@@ -45,6 +46,7 @@ impl Target {
             hir::ItemKind::Const(..) => Target::Const,
             hir::ItemKind::ForeignMod(..) => Target::ForeignMod,
             hir::ItemKind::Static(..) => Target::Static,
+            hir::ItemKind::Trait(..) => Target::Trait,
             _ => Target::Other,
         }
     }
@@ -70,6 +72,8 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
                 self.check_inline(attr, &item.span, target)
             } else if attr.check_name("non_exhaustive") {
                 self.check_non_exhaustive(attr, item, target)
+            } else if attr.check_name("marker") {
+                self.check_marker(attr, item, target)
             }
         }
 
@@ -114,6 +118,26 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
         }
     }
 
+    /// Check if the `#[marker]` attribute on an `item` is valid.
+    fn check_marker(&self, attr: &hir::Attribute, item: &hir::Item, target: Target) {
+        match target {
+            Target::Trait => { /* Valid */ },
+            _ => {
+                self.tcx.sess
+                    .struct_span_err(attr.span, "attribute can only be applied to a trait")
+                    .span_label(item.span, "not a trait")
+                    .emit();
+                return;
+            }
+        }
+
+        if !attr.is_word() {
+            self.tcx.sess
+                .struct_span_err(attr.span, "attribute should be empty")
+                .emit();
+        }
+    }
+
     /// Check if the `#[repr]` attributes on `item` are valid.
     fn check_repr(&self, item: &hir::Item, target: Target) {
         // Extract the names of all repr hints, e.g., [foo, bar, align] for:

src/librustc/ich/impls_ty.rs

@@ -1033,6 +1033,7 @@ impl_stable_hash_for!(struct ty::TraitDef {
     unsafety,
     paren_sugar,
     has_auto_impl,
+    is_marker,
     def_path_hash,
 });
 

src/librustc/ty/mod.rs

@@ -2662,23 +2662,32 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             as Box<dyn Iterator<Item = AssociatedItem> + 'a>
     }
 
-    /// Returns true if the impls are the same polarity and are implementing
-    /// a trait which contains no items
+    /// Returns true if the impls are the same polarity and the trait either
+    /// has no items or is annotated #[marker] and prevents item overrides.
     pub fn impls_are_allowed_to_overlap(self, def_id1: DefId, def_id2: DefId) -> bool {
-        if !self.features().overlapping_marker_traits {
-            return false;
+        if self.features().overlapping_marker_traits {
+            let trait1_is_empty = self.impl_trait_ref(def_id1)
+                .map_or(false, |trait_ref| {
+                    self.associated_item_def_ids(trait_ref.def_id).is_empty()
+                });
+            let trait2_is_empty = self.impl_trait_ref(def_id2)
+                .map_or(false, |trait_ref| {
+                    self.associated_item_def_ids(trait_ref.def_id).is_empty()
+                });
+            self.impl_polarity(def_id1) == self.impl_polarity(def_id2)
+                && trait1_is_empty
+                && trait2_is_empty
+        } else if self.features().marker_trait_attr {
+            let is_marker_impl = |def_id: DefId| -> bool {
+                let trait_ref = self.impl_trait_ref(def_id);
+                trait_ref.map_or(false, |tr| self.trait_def(tr.def_id).is_marker)
+            };
+            self.impl_polarity(def_id1) == self.impl_polarity(def_id2)
+                && is_marker_impl(def_id1)
+                && is_marker_impl(def_id2)
+        } else {
+            false
         }
-        let trait1_is_empty = self.impl_trait_ref(def_id1)
-            .map_or(false, |trait_ref| {
-                self.associated_item_def_ids(trait_ref.def_id).is_empty()
-            });
-        let trait2_is_empty = self.impl_trait_ref(def_id2)
-            .map_or(false, |trait_ref| {
-                self.associated_item_def_ids(trait_ref.def_id).is_empty()
-            });
-        self.impl_polarity(def_id1) == self.impl_polarity(def_id2)
-            && trait1_is_empty
-            && trait2_is_empty
     }
 
     // Returns `ty::VariantDef` if `def` refers to a struct,

src/librustc/ty/trait_def.rs

@@ -36,6 +36,11 @@ pub struct TraitDef {
 
     pub has_auto_impl: bool,
 
+    /// If `true`, then this trait has the `#[marker]` attribute, indicating
+    /// that all its associated items have defaults that cannot be overridden,
+    /// and thus `impl`s of it are allowed to overlap.
+    pub is_marker: bool,
+
     /// The ICH of this trait's DefPath, cached here so it doesn't have to be
     /// recomputed all the time.
     pub def_path_hash: DefPathHash,
@@ -53,13 +58,15 @@ impl<'a, 'gcx, 'tcx> TraitDef {
                unsafety: hir::Unsafety,
                paren_sugar: bool,
                has_auto_impl: bool,
+               is_marker: bool,
                def_path_hash: DefPathHash)
                -> TraitDef {
         TraitDef {
             def_id,
-            paren_sugar,
             unsafety,
+            paren_sugar,
             has_auto_impl,
+            is_marker,
             def_path_hash,
         }
     }

src/librustc_metadata/decoder.rs

@@ -539,6 +539,7 @@ impl<'a, 'tcx> CrateMetadata {
                           data.unsafety,
                           data.paren_sugar,
                           data.has_auto_impl,
+                          data.is_marker,
                           self.def_path_table.def_path_hash(item_id))
     }
 

src/librustc_metadata/encoder.rs

@@ -1152,6 +1152,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
                     unsafety: trait_def.unsafety,
                     paren_sugar: trait_def.paren_sugar,
                     has_auto_impl: tcx.trait_is_auto(def_id),
+                    is_marker: trait_def.is_marker,
                     super_predicates: self.lazy(&tcx.super_predicates_of(def_id)),
                 };
 

src/librustc_metadata/schema.rs

@@ -472,13 +472,15 @@ pub struct TraitData<'tcx> {
     pub unsafety: hir::Unsafety,
     pub paren_sugar: bool,
     pub has_auto_impl: bool,
+    pub is_marker: bool,
     pub super_predicates: Lazy<ty::GenericPredicates<'tcx>>,
 }
 
 impl_stable_hash_for!(struct TraitData<'tcx> {
     unsafety,
     paren_sugar,
     has_auto_impl,
+    is_marker,
     super_predicates
 });
 

src/librustc_typeck/check/wfcheck.rs

@@ -304,6 +304,19 @@ fn check_type_defn<'a, 'tcx, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 
 fn check_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item: &hir::Item) {
     let trait_def_id = tcx.hir.local_def_id(item.id);
+
+    let trait_def = tcx.trait_def(trait_def_id);
+    if trait_def.is_marker {
+        for associated_def_id in &*tcx.associated_item_def_ids(trait_def_id) {
+            struct_span_err!(
+                tcx.sess,
+                tcx.def_span(*associated_def_id),
+                E0714,
+                "marker traits cannot have associated items",
+            ).emit();
+        }
+    }
+
     for_item(tcx, item).with_fcx(|fcx, _| {
         check_where_clauses(tcx, fcx, item.span, trait_def_id, None);
         vec![]

src/librustc_typeck/coherence/mod.rs

@@ -46,6 +46,7 @@ fn check_impl<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, node_id: ast::NodeId) {
         }
 
         enforce_trait_manually_implementable(tcx, impl_def_id, trait_ref.def_id);
+        enforce_empty_impls_for_marker_traits(tcx, impl_def_id, trait_ref.def_id);
     }
 }
 
@@ -99,6 +100,25 @@ fn enforce_trait_manually_implementable(tcx: TyCtxt, impl_def_id: DefId, trait_d
         .emit();
 }
 
+/// We allow impls of marker traits to overlap, so they can't override impls
+/// as that could make it ambiguous which associated item to use.
+fn enforce_empty_impls_for_marker_traits(tcx: TyCtxt, impl_def_id: DefId, trait_def_id: DefId) {
+    if !tcx.trait_def(trait_def_id).is_marker {
+        return;
+    }
+
+    if tcx.associated_item_def_ids(trait_def_id).is_empty() {
+        return;
+    }
+
+    let span = tcx.sess.source_map().def_span(tcx.span_of_impl(impl_def_id).unwrap());
+    struct_span_err!(tcx.sess,
+                     span,
+                     E0715,
+                     "impls for marker traits cannot contain items")
+        .emit();
+}
+
 pub fn provide(providers: &mut Providers) {
     use self::builtin::coerce_unsized_info;
     use self::inherent_impls::{crate_inherent_impls, inherent_impls};

src/librustc_typeck/collect.rs

@@ -740,8 +740,9 @@ fn trait_def<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx ty::
         err.emit();
     }
 
+    let is_marker = tcx.has_attr(def_id, "marker");
     let def_path_hash = tcx.def_path_hash(def_id);
-    let def = ty::TraitDef::new(def_id, unsafety, paren_sugar, is_auto, def_path_hash);
+    let def = ty::TraitDef::new(def_id, unsafety, paren_sugar, is_auto, is_marker, def_path_hash);
     tcx.alloc_trait_def(def)
 }
 

src/librustc_typeck/diagnostics.rs

@@ -4750,6 +4750,22 @@ ambiguity for some types, we disallow calling methods on raw pointers when
 the type is unknown.
 "##,
 
+E0714: r##"
+A `#[marker]` trait contained an associated item.
+
+The items of marker traits cannot be overridden, so there's no need to have them
+when they cannot be changed per-type anyway.  If you wanted them for ergonomic
+reasons, consider making an extension trait instead.
+"##,
+
+E0715: r##"
+An `impl` for a `#[marker]` trait tried to override an associated item.
+
+Because marker traits are allowed to have multiple implementations for the same
+type, it's not allowed to override anything in those implementations, as it
+would be ambiguous which override should actually be used.
+"##,
+
 }
 
 register_diagnostics! {

src/libsyntax/feature_gate.rs

@@ -355,6 +355,9 @@ declare_features! (
     // Allows overlapping impls of marker traits
     (active, overlapping_marker_traits, "1.18.0", Some(29864), None),
 
+    // Trait attribute to allow overlapping impls
+    (active, marker_trait_attr, "1.30.0", Some(29864), None),
+
     // rustc internal
     (active, abi_thiscall, "1.19.0", None, None),
 
@@ -808,6 +811,12 @@ pub const BUILTIN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeG
                                           "non exhaustive is an experimental feature",
                                           cfg_fn!(non_exhaustive))),
 
+    // RFC #1268
+    ("marker", Normal, Gated(Stability::Unstable,
+                             "marker_trait_attr",
+                             "marker traits is an experimental feature",
+                             cfg_fn!(marker_trait_attr))),
+
     ("plugin", CrateLevel, Gated(Stability::Unstable,
                                  "plugin",
                                  "compiler plugins are experimental \

src/test/run-pass/overlap-permitted-for-annotated-marker-traits.rs

@@ -0,0 +1,35 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Tests for RFC 1268: we allow overlapping impls of marker traits,
+// that is, traits with #[marker]. In this case, a type `T` is
+// `MyMarker` if it is either `Debug` or `Display`.
+
+#![feature(marker_trait_attr)]
+
+use std::fmt::{Debug, Display};
+
+#[marker] trait MyMarker {}
+
+impl<T: Debug> MyMarker for T {}
+impl<T: Display> MyMarker for T {}
+
+fn foo<T: MyMarker>(t: T) -> T {
+    t
+}
+
+fn main() {
+    // Debug && Display:
+    assert_eq!(1, foo(1));
+    assert_eq!(2.0, foo(2.0));
+
+    // Debug && !Display:
+    assert_eq!(vec![1], foo(vec![1]));
+}

src/test/ui/feature-gates/feature-gate-marker_trait_attr.rs

@@ -0,0 +1,19 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::fmt::{Debug, Display};
+
+#[marker] trait ExplicitMarker {}
+//~^ ERROR marker traits is an experimental feature (see issue #29864)
+
+impl<T: Display> ExplicitMarker for T {}
+impl<T: Debug> ExplicitMarker for T {}
+
+fn main() {}

src/test/ui/feature-gates/feature-gate-marker_trait_attr.stderr

@@ -0,0 +1,11 @@
+error[E0658]: marker traits is an experimental feature (see issue #29864)
+  --> $DIR/feature-gate-marker_trait_attr.rs:13:1
+   |
+LL | #[marker] trait ExplicitMarker {}
+   | ^^^^^^^^^
+   |
+   = help: add #![feature(marker_trait_attr)] to the crate attributes to enable
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0658`.