Address code review comments

Author: eholk

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -19,7 +19,6 @@ use rustc_session::lint::builtin::{
     DEPRECATED_WHERE_CLAUSE_LOCATION, MISSING_ABI, PATTERNS_IN_FNS_WITHOUT_BODY,
 };
 use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer};
-use rustc_session::parse::feature_err;
 use rustc_session::Session;
 use rustc_span::source_map::Spanned;
 use rustc_span::symbol::{kw, sym, Ident};
@@ -754,19 +753,7 @@ impl<'a> AstValidator<'a> {
                     self.maybe_lint_missing_abi(sig_span, ty.id);
                 }
             }
-            TyKind::TraitObject(ref bounds, syntax, ..) => {
-                if syntax == TraitObjectSyntax::DynStar
-                    && !self.session.features_untracked().dyn_star
-                {
-                    feature_err(
-                        &self.session.parse_sess,
-                        sym::dyn_star,
-                        ty.span,
-                        "dyn* trait objects are unstable",
-                    )
-                    .emit();
-                }
-
+            TyKind::TraitObject(ref bounds, ..) => {
                 let mut any_lifetime_bounds = false;
                 for bound in bounds {
                     if let GenericBound::Outlives(ref lifetime) = *bound {

compiler/rustc_ast_passes/src/feature_gate.rs

@@ -573,6 +573,9 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
             ast::TyKind::Never => {
                 gate_feature_post!(&self, never_type, ty.span, "the `!` type is experimental");
             }
+            ast::TyKind::TraitObject(_, ast::TraitObjectSyntax::DynStar, ..) => {
+                gate_feature_post!(&self, dyn_star, ty.span, "dyn* trait objects are unstable");
+            }
             _ => {}
         }
         visit::walk_ty(self, ty)

compiler/rustc_codegen_cranelift/src/value_and_place.rs

@@ -816,6 +816,7 @@ pub(crate) fn assert_assignable<'tcx>(
             // fn(&T) -> for<'l> fn(&'l T) is allowed
         }
         (&ty::Dynamic(from_traits, _, _from_kind), &ty::Dynamic(to_traits, _, _to_kind)) => {
+            // FIXME(dyn-star): Do the right thing with DynKinds
             for (from, to) in from_traits.iter().zip(to_traits) {
                 let from =
                     fx.tcx.normalize_erasing_late_bound_regions(ParamEnv::reveal_all(), from);

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -367,7 +367,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         bx.ret(llval);
     }
 
-    #[tracing::instrument(level = "debug", skip(self, helper, bx))]
+    #[tracing::instrument(level = "trace", skip(self, helper, bx))]
     fn codegen_drop_terminator(
         &mut self,
         helper: TerminatorCodegenHelper<'tcx>,

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -113,11 +113,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 if let ty::Dynamic(data, _, ty::DynStar) = cast_ty.kind() {
                     // Initial cast from sized to dyn trait
                     let vtable = self.get_vtable_ptr(src.layout.ty, data.principal())?;
-                    let ptr = self.read_immediate(src)?.to_scalar();
-                    // FIXME(dyn-star): This should not use new_dyn_trait, but
-                    // it does exactly the same thing (makes a scalar pair)...
-                    // so maybe we should just duplicate/rename the function.
-                    let val = Immediate::new_dyn_trait(ptr, vtable, &*self.tcx);
+                    let vtable = Scalar::from_maybe_pointer(vtable, self);
+                    let data = self.read_immediate(src)?.to_scalar();
+                    let _assert_pointer_sized = data.to_pointer(self)?;
+                    let val = Immediate::ScalarPair(data, vtable);
                     self.write_immediate(val, dest)?;
                 } else {
                     bug!()
@@ -327,7 +326,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 let new_vptr = self.get_vtable_ptr(ty, data_b.principal())?;
                 self.write_immediate(Immediate::new_dyn_trait(old_data, new_vptr, self), dest)
             }
-            (_, &ty::Dynamic(ref data, _, _repr)) => {
+            (_, &ty::Dynamic(ref data, _, ty::Dyn)) => {
                 // Initial cast from sized to dyn trait
                 let vtable = self.get_vtable_ptr(src_pointee_ty, data.principal())?;
                 let ptr = self.read_scalar(src)?;

compiler/rustc_const_eval/src/transform/validate.rs

@@ -570,7 +570,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                         }
                     }
                     CastKind::DynStar => {
-                        // FIXME: make sure nothing needs to be done here.
+                        // FIXME(dyn-star): make sure nothing needs to be done here.
                     }
                     // Nothing to check here
                     CastKind::PointerFromExposedAddress

compiler/rustc_feature/src/active.rs

@@ -381,7 +381,7 @@ declare_features! (
     /// Allows `#[doc(masked)]`.
     (active, doc_masked, "1.21.0", Some(44027), None),
     /// Allows `dyn* Trait` objects.
-    (active, dyn_star, "1.65.0", Some(91611), None),
+    (incomplete, dyn_star, "CURRENT_RUSTC_VERSION", Some(91611), None),
     /// Allows `X..Y` patterns.
     (active, exclusive_range_pattern, "1.11.0", Some(37854), None),
     /// Allows exhaustive pattern matching on types that contain uninhabited types.

compiler/rustc_middle/src/ty/layout.rs

@@ -626,11 +626,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
             }
 
             ty::Dynamic(_, _, ty::DynStar) => {
-                let mut pointer = scalar_unit(Int(dl.ptr_sized_integer(), false));
-                pointer.valid_range_mut().start = 1;
+                let mut data = scalar_unit(Int(dl.ptr_sized_integer(), false));
+                data.valid_range_mut().start = 0;
                 let mut vtable = scalar_unit(Pointer);
                 vtable.valid_range_mut().start = 1;
-                tcx.intern_layout(self.scalar_pair(pointer, vtable))
+                tcx.intern_layout(self.scalar_pair(data, vtable))
             }
 
             // Arrays and slices.
@@ -2474,8 +2474,7 @@ where
 
                     match tcx.struct_tail_erasing_lifetimes(pointee, cx.param_env()).kind() {
                         ty::Slice(_) | ty::Str => TyMaybeWithLayout::Ty(tcx.types.usize),
-                        // FIXME(eholk): Do the right thing with trait object representation
-                        ty::Dynamic(_, _, _repr) => {
+                        ty::Dynamic(_, _, ty::Dyn) => {
                             TyMaybeWithLayout::Ty(tcx.mk_imm_ref(
                                 tcx.lifetimes.re_static,
                                 tcx.mk_array(tcx.types.usize, 3),
@@ -3379,7 +3378,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
         Ok(self.tcx.arena.alloc(fn_abi))
     }
 
-    #[tracing::instrument(level = "debug", skip(self))]
+    #[tracing::instrument(level = "trace", skip(self))]
     fn fn_abi_adjust_for_abi(
         &self,
         fn_abi: &mut FnAbi<'tcx, Ty<'tcx>>,

compiler/rustc_monomorphize/src/collector.rs

@@ -699,7 +699,9 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
                 // This could also be a different Unsize instruction, like
                 // from a fixed sized array to a slice. But we are only
                 // interested in things that produce a vtable.
-                if (target_ty.is_trait() || target_ty.is_dyn_star()) && !source_ty.is_trait() {
+                if (target_ty.is_trait() && !source_ty.is_trait())
+                    || (target_ty.is_dyn_star() && !source_ty.is_dyn_star())
+                {
                     create_mono_items_for_vtable_methods(
                         self.tcx,
                         target_ty,

compiler/rustc_parse/src/parser/ty.rs

@@ -579,9 +579,7 @@ impl<'a> Parser<'a> {
         self.bump(); // `dyn`
 
         // parse dyn* types
-        let dyn_star = matches!(self.token.kind, TokenKind::BinOp(token::Star));
-        let syntax = if dyn_star {
-            self.bump(); // `*`
+        let syntax = if self.eat(&TokenKind::BinOp(token::Star)) {
             TraitObjectSyntax::DynStar
         } else {
             TraitObjectSyntax::Dyn

compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs

@@ -627,10 +627,13 @@ fn encode_ty<'tcx>(
         }
 
         // Trait types
-        ty::Dynamic(predicates, region, _repr) => {
+        ty::Dynamic(predicates, region, kind) => {
             // u3dynI<element-type1[..element-typeN]>E, where <element-type> is <predicate>, as
             // vendor extended type.
-            let mut s = String::from("u3dynI");
+            let mut s = String::from(match kind {
+                ty::Dyn => "u3dynI",
+                ty::DynStar => "u7dynstarI",
+            });
             s.push_str(&encode_predicates(tcx, predicates, dict, options));
             s.push_str(&encode_region(tcx, *region, dict, options));
             s.push('E');

compiler/rustc_symbol_mangling/src/v0.rs

@@ -479,8 +479,12 @@ impl<'tcx> Printer<'tcx> for &mut SymbolMangler<'tcx> {
                 })?;
             }
 
-            ty::Dynamic(predicates, r, _repr) => {
-                self.push("D");
+            ty::Dynamic(predicates, r, kind) => {
+                self.push(match kind {
+                    ty::Dyn => "D",
+                    // FIXME(dyn-star): need to update v0 mangling docs
+                    ty::DynStar => "D*",
+                });
                 self = self.print_dyn_existential(predicates)?;
                 self = r.print(self)?;
             }

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -1067,7 +1067,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         self_ty: Ty<'tcx>,
         object_ty: Ty<'tcx>,
     ) {
-        let ty::Dynamic(predicates, _, _) = object_ty.kind() else { return; };
+        let ty::Dynamic(predicates, _, ty::Dyn) = object_ty.kind() else { return; };
         let self_ref_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_erased, self_ty);
 
         for predicate in predicates.iter() {
@@ -1365,7 +1365,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         let trait_pred = self.resolve_vars_if_possible(trait_pred);
         let ty = trait_pred.skip_binder().self_ty();
         let is_object_safe = match ty.kind() {
-            ty::Dynamic(predicates, _, _) => {
+            ty::Dynamic(predicates, _, ty::Dyn) => {
                 // If the `dyn Trait` is not object safe, do not suggest `Box<dyn Trait>`.
                 predicates
                     .principal_def_id()

compiler/rustc_trait_selection/src/traits/select/confirmation.rs

@@ -890,9 +890,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         let mut nested = vec![];
         match (source.kind(), target.kind()) {
             // Trait+Kx+'a -> Trait+Ky+'b (auto traits and lifetime subtyping).
-            (&ty::Dynamic(ref data_a, r_a, repr_a), &ty::Dynamic(ref data_b, r_b, repr_b))
-                if repr_a == repr_b =>
-            {
+            (&ty::Dynamic(ref data_a, r_a, ty::Dyn), &ty::Dynamic(ref data_b, r_b, ty::Dyn)) => {
                 // See `assemble_candidates_for_unsizing` for more info.
                 // We already checked the compatibility of auto traits within `assemble_candidates_for_unsizing`.
                 let iter = data_a
@@ -911,7 +909,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                             .map(ty::Binder::dummy),
                     );
                 let existential_predicates = tcx.mk_poly_existential_predicates(iter);
-                let source_trait = tcx.mk_dynamic(existential_predicates, r_b, repr_b);
+                let source_trait = tcx.mk_dynamic(existential_predicates, r_b, ty::Dyn);
 
                 // Require that the traits involved in this upcast are **equal**;
                 // only the **lifetime bound** is changed.
@@ -938,7 +936,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             }
 
             // `T` -> `Trait`
-            (_, &ty::Dynamic(ref data, r, _repr)) => {
+            (_, &ty::Dynamic(ref data, r, ty::Dyn)) => {
                 let mut object_dids = data.auto_traits().chain(data.principal_def_id());
                 if let Some(did) = object_dids.find(|did| !tcx.is_object_safe(*did)) {
                     return Err(TraitNotObjectSafe(did));

compiler/rustc_traits/src/chalk/lowering.rs

@@ -326,7 +326,8 @@ impl<'tcx> LowerInto<'tcx, chalk_ir::Ty<RustInterner<'tcx>>> for Ty<'tcx> {
                     )),
                 })
             }
-            ty::Dynamic(predicates, region, _repr) => chalk_ir::TyKind::Dyn(chalk_ir::DynTy {
+            // FIXME(dyn-star): handle the dynamic kind (dyn or dyn*)
+            ty::Dynamic(predicates, region, _kind) => chalk_ir::TyKind::Dyn(chalk_ir::DynTy {
                 bounds: predicates.lower_into(interner),
                 lifetime: region.lower_into(interner),
             }),

compiler/rustc_type_ir/src/sty.rs

@@ -19,26 +19,33 @@ use rustc_data_structures::stable_hasher::HashStable;
 use rustc_serialize::{Decodable, Decoder, Encodable};
 
 /// Specifies how a trait object is represented.
-#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Encodable, Decodable)]
+#[derive(
+    Clone,
+    Copy,
+    PartialEq,
+    Eq,
+    PartialOrd,
+    Ord,
+    Hash,
+    Debug,
+    Encodable,
+    Decodable,
+    HashStable_Generic
+)]
 pub enum DynKind {
     /// An unsized `dyn Trait` object
     Dyn,
     /// A sized `dyn* Trait` object
+    ///
+    /// These objects are represented as a `(data, vtable)` pair where `data` is a ptr-sized value
+    /// (often a pointer to the real object, but not necessarily) and `vtable` is a pointer to
+    /// the vtable for `dyn* Trait`. The representation is essentially the same as `&dyn Trait`
+    /// or similar, but the drop function included in the vtable is responsible for freeing the
+    /// underlying storage if needed. This allows a `dyn*` object to be treated agnostically with
+    /// respect to whether it points to a `Box<T>`, `Rc<T>`, etc.
     DynStar,
 }
 
-// Manually implemented because deriving HashStable requires rustc_query_system, which would
-// create a cyclic dependency.
-impl<CTX> HashStable<CTX> for DynKind {
-    fn hash_stable(
-        &self,
-        hcx: &mut CTX,
-        hasher: &mut rustc_data_structures::stable_hasher::StableHasher,
-    ) {
-        std::mem::discriminant(self).hash_stable(hcx, hasher);
-    }
-}
-
 /// Defines the kinds of types used by the type system.
 ///
 /// Types written by the user start out as `hir::TyKind` and get

compiler/rustc_typeck/src/check/cast.rs

@@ -105,7 +105,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         Ok(match *t.kind() {
             ty::Slice(_) | ty::Str => Some(PointerKind::Length),
-            ty::Dynamic(ref tty, ..) => Some(PointerKind::VTable(tty.principal_def_id())),
+            ty::Dynamic(ref tty, _, ty::Dyn) => Some(PointerKind::VTable(tty.principal_def_id())),
             ty::Adt(def, substs) if def.is_struct() => match def.non_enum_variant().fields.last() {
                 None => Some(PointerKind::Thin),
                 Some(f) => {
@@ -142,6 +142,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             | ty::Generator(..)
             | ty::Adt(..)
             | ty::Never
+            | ty::Dynamic(_, _, ty::DynStar)
             | ty::Error(_) => {
                 let reported = self
                     .tcx
@@ -246,7 +247,7 @@ fn check_dyn_star_cast<'tcx>(
     let cause = ObligationCause::new(
         expr.span,
         fcx.body_id,
-        // FIXME: Use a better obligation cause code
+        // FIXME(dyn-star): Use a better obligation cause code
         ObligationCauseCode::MiscObligation,
     );
 
@@ -927,10 +928,10 @@ impl<'a, 'tcx> CastCheck<'tcx> {
 
             (Int(_) | Float, Int(_) | Float) => Ok(CastKind::NumericCast),
 
-            // FIXME: this needs more conditions...
+            // FIXME(dyn-star): this needs more conditions...
             (_, DynStar) => Ok(CastKind::DynStarCast),
 
-            // FIXME: do we want to allow dyn* upcasting or other casts?
+            // FIXME(dyn-star): do we want to allow dyn* upcasting or other casts?
             (DynStar, _) => Err(CastError::IllegalCast),
         }
     }

src/test/ui/dyn-star/const.rs

@@ -1,6 +1,6 @@
 // run-pass
 #![feature(dyn_star)]
-#![allow(unused)]
+#![allow(unused, incomplete_features)]
 
 use std::fmt::Debug;
 

src/test/ui/dyn-star/drop.rs

@@ -1,6 +1,7 @@
 // run-pass
 // check-run-results
 #![feature(dyn_star)]
+#![allow(incomplete_features)]
 
 use std::fmt::Debug;
 

src/test/ui/dyn-star/error.rs

@@ -1,4 +1,5 @@
 #![feature(dyn_star)]
+#![allow(incomplete_features)]
 
 use std::fmt::Debug;
 

src/test/ui/dyn-star/error.stderr

@@ -1,5 +1,5 @@
 error[E0277]: the trait bound `{integer}: Foo` is not satisfied
-  --> $DIR/error.rs:9:27
+  --> $DIR/error.rs:10:27
    |
 LL |     let dyn_i: dyn* Foo = i as dyn* Foo;
    |                           ^ the trait `Foo` is not implemented for `{integer}`

src/test/ui/dyn-star/make-dyn-star.rs

@@ -1,5 +1,6 @@
 // run-pass
 #![feature(dyn_star)]
+#![allow(incomplete_features)]
 
 use std::fmt::Debug;
 

src/test/ui/dyn-star/method.rs

@@ -1,5 +1,6 @@
 // run-pass
 #![feature(dyn_star)]
+#![allow(incomplete_features)]
 
 trait Foo {
     fn get(&self) -> usize;

src/test/ui/dyn-star/syntax.rs

@@ -3,6 +3,7 @@
 // check-pass
 
 #![feature(dyn_star)]
+#![allow(incomplete_features)]
 
 pub fn dyn_star_parameter(_: dyn* Send) {
 }