Auto merge of #44456 - eddyb:stable-drop-const, r=nikomatsakis

Stabilize drop_types_in_const.

Closes #33156, stabilizing the new, revised, rules, and improving the error message.

r? @nikomatsakis cc @SergioBenitez

Author: bors

src/librustc_mir/diagnostics.rs

@@ -431,29 +431,6 @@ Remember this solution is unsafe! You will have to ensure that accesses to the
 cell are synchronized.
 "##,
 
-E0493: r##"
-A type with a destructor was assigned to an invalid type of variable. Erroneous
-code example:
-
-```compile_fail,E0493
-struct Foo {
-    a: u32
-}
-
-impl Drop for Foo {
-    fn drop(&mut self) {}
-}
-
-const F : Foo = Foo { a : 0 };
-// error: constants are not allowed to have destructors
-static S : Foo = Foo { a : 0 };
-// error: destructors in statics are an unstable feature
-```
-
-To solve this issue, please use a type which does allow the usage of type with
-destructors.
-"##,
-
 E0494: r##"
 A reference of an interior static was assigned to another const/static.
 Erroneous code example:
@@ -991,6 +968,7 @@ fn print_fancy_ref(fancy_ref: &FancyNum){
 }
 
 register_diagnostics! {
+    E0493, // destructors cannot be evaluated at compile-time
     E0524, // two closures require unique access to `..` at the same time
     E0526, // shuffle indices are not constant
     E0625, // thread-local statics cannot be accessed at compile-time

src/librustc_mir/transform/qualify_consts.rs

@@ -18,7 +18,6 @@ use rustc_data_structures::bitvec::BitVector;
 use rustc_data_structures::indexed_set::IdxSetBuf;
 use rustc_data_structures::indexed_vec::{IndexVec, Idx};
 use rustc::hir;
-use rustc::hir::map as hir_map;
 use rustc::hir::def_id::DefId;
 use rustc::middle::const_val::ConstVal;
 use rustc::traits::{self, Reveal};
@@ -197,91 +196,6 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
         self.add(original);
     }
 
-    /// Check for NEEDS_DROP (from an ADT or const fn call) and
-    /// error, unless we're in a function.
-    fn always_deny_drop(&self) {
-        self.deny_drop_with_feature_gate_override(false);
-    }
-
-    /// Check for NEEDS_DROP (from an ADT or const fn call) and
-    /// error, unless we're in a function, or the feature-gate
-    /// for constant with destructors is enabled.
-    fn deny_drop(&self) {
-        self.deny_drop_with_feature_gate_override(true);
-    }
-
-    fn deny_drop_with_feature_gate_override(&self, allow_gate: bool) {
-        if self.mode == Mode::Fn || !self.qualif.intersects(Qualif::NEEDS_DROP) {
-            return;
-        }
-
-        // Constants allow destructors, but they're feature-gated.
-        let msg = if allow_gate {
-            // Feature-gate for constant with destructors is enabled.
-            if self.tcx.sess.features.borrow().drop_types_in_const {
-                return;
-            }
-
-            // This comes from a macro that has #[allow_internal_unstable].
-            if self.span.allows_unstable() {
-                return;
-            }
-
-            format!("destructors in {}s are an unstable feature",
-                    self.mode)
-        } else {
-            format!("{}s are not allowed to have destructors",
-                    self.mode)
-        };
-
-        let mut err =
-            struct_span_err!(self.tcx.sess, self.span, E0493, "{}", msg);
-
-        if allow_gate {
-            help!(&mut err,
-                  "in Nightly builds, add `#![feature(drop_types_in_const)]` \
-                   to the crate attributes to enable");
-        } else {
-            // FIXME(eddyb) this looks up `self.mir.return_ty`.
-            // We probably want the actual return type here, if at all.
-            self.find_drop_implementation_method_span()
-                .map(|span| err.span_label(span, "destructor defined here"));
-
-            err.span_label(self.span,
-                format!("{}s cannot have destructors", self.mode));
-        }
-
-        err.emit();
-    }
-
-    fn find_drop_implementation_method_span(&self) -> Option<Span> {
-        self.tcx.lang_items()
-            .drop_trait()
-            .and_then(|drop_trait_id| {
-                let mut span = None;
-
-                self.tcx
-                    .for_each_relevant_impl(drop_trait_id, self.mir.return_ty, |impl_did| {
-                        self.tcx.hir
-                            .as_local_node_id(impl_did)
-                            .and_then(|impl_node_id| self.tcx.hir.find(impl_node_id))
-                            .map(|node| {
-                                if let hir_map::NodeItem(item) = node {
-                                    if let hir::ItemImpl(.., ref impl_item_refs) = item.node {
-                                        span = impl_item_refs.first()
-                                                             .map(|iiref| {
-                                                                 self.tcx.hir.impl_item(iiref.id)
-                                                                             .span
-                                                             });
-                                    }
-                                }
-                            });
-                    });
-
-                span
-            })
-    }
-
     /// Check if an Lvalue with the current qualifications could
     /// be consumed, by either an operand or a Deref projection.
     fn try_consume(&mut self) -> bool {
@@ -458,25 +372,17 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
             }
         }
 
-        let return_ty = mir.return_ty;
         self.qualif = self.return_qualif.unwrap_or(Qualif::NOT_CONST);
 
-        match self.mode {
-            Mode::StaticMut => {
-                // Check for destructors in static mut.
-                self.add_type(return_ty);
-                self.deny_drop();
-            }
-            _ => {
-                // Account for errors in consts by using the
-                // conservative type qualification instead.
-                if self.qualif.intersects(Qualif::CONST_ERROR) {
-                    self.qualif = Qualif::empty();
-                    self.add_type(return_ty);
-                }
-            }
+        // Account for errors in consts by using the
+        // conservative type qualification instead.
+        if self.qualif.intersects(Qualif::CONST_ERROR) {
+            self.qualif = Qualif::empty();
+            let return_ty = mir.return_ty;
+            self.add_type(return_ty);
         }
 
+
         // Collect all the temps we need to promote.
         let mut promoted_temps = IdxSetBuf::new_empty(self.temp_promotion_state.len());
 
@@ -640,12 +546,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                         // with type parameters, take it into account.
                         self.qualif.restrict(ty, self.tcx, self.param_env);
                     }
-
-                    // Let `const fn` transitively have destructors,
-                    // but they do get stopped in `const` or `static`.
-                    if self.mode != Mode::ConstFn {
-                        self.deny_drop();
-                    }
                 }
             }
         }
@@ -690,12 +590,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                     let allow = if self.mode == Mode::StaticMut {
                         // Inside a `static mut`, &mut [...] is also allowed.
                         match ty.sty {
-                            ty::TyArray(..) | ty::TySlice(_) => {
-                                // Mutating can expose drops, be conservative.
-                                self.add_type(ty);
-                                self.deny_drop();
-                                true
-                            }
+                            ty::TyArray(..) | ty::TySlice(_) => true,
                             _ => false
                         }
                     } else if let ty::TyArray(_, len) = ty.sty {
@@ -798,18 +693,12 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                 if let AggregateKind::Adt(def, ..) = **kind {
                     if def.has_dtor(self.tcx) {
                         self.add(Qualif::NEEDS_DROP);
-                        self.deny_drop();
                     }
 
                     if Some(def.did) == self.tcx.lang_items().unsafe_cell_type() {
                         let ty = rvalue.ty(self.mir, self.tcx);
                         self.add_type(ty);
                         assert!(self.qualif.intersects(Qualif::MUTABLE_INTERIOR));
-                        // Even if the value inside may not need dropping,
-                        // mutating it would change that.
-                        if !self.qualif.intersects(Qualif::NOT_CONST) {
-                            self.deny_drop();
-                        }
                     }
                 }
             }
@@ -919,12 +808,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                     let ty = dest.ty(self.mir, tcx).to_ty(tcx);
                     self.qualif = Qualif::empty();
                     self.add_type(ty);
-
-                    // Let `const fn` transitively have destructors,
-                    // but they do get stopped in `const` or `static`.
-                    if self.mode != Mode::ConstFn {
-                        self.deny_drop();
-                    }
                 }
                 self.assign(dest, location);
             }
@@ -942,14 +825,15 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                 };
 
                 if let Some(span) = needs_drop {
+                    // Double-check the type being dropped, to minimize false positives.
                     let ty = lvalue.ty(self.mir, self.tcx).to_ty(self.tcx);
-                    self.add_type(ty);
-
-                    // Use the original assignment span to be more precise.
-                    let old_span = self.span;
-                    self.span = span;
-                    self.always_deny_drop();
-                    self.span = old_span;
+                    if ty.needs_drop(self.tcx, self.param_env) {
+                        struct_span_err!(self.tcx.sess, span, E0493,
+                                         "destructors cannot be evaluated at compile-time")
+                            .span_label(span, format!("{}s cannot evaluate destructors",
+                                                      self.mode))
+                            .emit();
+                    }
                 }
             }
         } else {

src/libsyntax/feature_gate.rs

@@ -269,9 +269,6 @@ declare_features! (
     // impl specialization (RFC 1210)
     (active, specialization, "1.7.0", Some(31844)),
 
-    // Allow Drop types in statics/const functions (RFC 1440)
-    (active, drop_types_in_const, "1.9.0", Some(33156)),
-
     // Allows cfg(target_has_atomic = "...").
     (active, cfg_target_has_atomic, "1.9.0", Some(32976)),
 
@@ -469,6 +466,8 @@ declare_features! (
     (accepted, compile_error, "1.20.0", Some(40872)),
     // See rust-lang/rfcs#1414. Allows code like `let x: &'static u32 = &42` to work.
     (accepted, rvalue_static_promotion, "1.21.0", Some(38865)),
+    // Allow Drop types in constants (RFC 1440)
+    (accepted, drop_types_in_const, "1.22.0", Some(33156)),
 );
 
 // If you change this, please modify src/doc/unstable-book as well. You must

src/test/compile-fail/check-static-values-constraints.rs

@@ -10,8 +10,6 @@
 
 // Verifies all possible restrictions for statics values.
 
-// gate-test-drop_types_in_const
-
 #![allow(warnings)]
 #![feature(box_syntax)]
 
@@ -37,15 +35,8 @@ enum SafeEnum {
 // These should be ok
 static STATIC1: SafeEnum = SafeEnum::Variant1;
 static STATIC2: SafeEnum = SafeEnum::Variant2(0);
-
-// This one should fail
 static STATIC3: SafeEnum = SafeEnum::Variant3(WithDtor);
-//~^ ERROR destructors in statics are an unstable feature
-
 
-// This enum will be used to test that variants
-// are considered unsafe if their enum type implements
-// a destructor.
 enum UnsafeEnum {
     Variant5,
     Variant6(isize)
@@ -57,9 +48,7 @@ impl Drop for UnsafeEnum {
 
 
 static STATIC4: UnsafeEnum = UnsafeEnum::Variant5;
-//~^ ERROR destructors in statics are an unstable feature
 static STATIC5: UnsafeEnum = UnsafeEnum::Variant6(0);
-//~^ ERROR destructors in statics are an unstable feature
 
 
 struct SafeStruct {
@@ -71,10 +60,8 @@ struct SafeStruct {
 // Struct fields are safe, hence this static should be safe
 static STATIC6: SafeStruct = SafeStruct{field1: SafeEnum::Variant1, field2: SafeEnum::Variant2(0)};
 
-// field2 has an unsafe value, hence this should fail
 static STATIC7: SafeStruct = SafeStruct{field1: SafeEnum::Variant1,
                                         field2: SafeEnum::Variant3(WithDtor)};
-//~^ ERROR destructors in statics are an unstable feature
 
 // Test variadic constructor for structs. The base struct should be examined
 // as well as every field present in the constructor.
@@ -86,8 +73,7 @@ static STATIC8: SafeStruct = SafeStruct{field1: SafeEnum::Variant1,
 // This example should fail because field1 in the base struct is not safe
 static STATIC9: SafeStruct = SafeStruct{field1: SafeEnum::Variant1,
                                         ..SafeStruct{field1: SafeEnum::Variant3(WithDtor),
-//~^ ERROR destructors in statics are an unstable feature
-//~| ERROR statics are not allowed to have destructors
+//~^ ERROR destructors cannot be evaluated at compile-time
                                                      field2: SafeEnum::Variant1}};
 
 struct UnsafeStruct;
@@ -96,29 +82,19 @@ impl Drop for UnsafeStruct {
     fn drop(&mut self) {}
 }
 
-// Types with destructors are not allowed for statics
 static STATIC10: UnsafeStruct = UnsafeStruct;
-//~^ ERROR destructors in statics are an unstable feature
 
 struct MyOwned;
 
 static STATIC11: Box<MyOwned> = box MyOwned;
 //~^ ERROR allocations are not allowed in statics
 
-// The following examples test that mutable structs are just forbidden
-// to have types with destructors
-// These should fail
 static mut STATIC12: UnsafeStruct = UnsafeStruct;
-//~^ ERROR destructors in statics are an unstable feature
-//~^^ ERROR destructors in statics are an unstable feature
 
 static mut STATIC13: SafeStruct = SafeStruct{field1: SafeEnum::Variant1,
-//~^ ERROR destructors in statics are an unstable feature
                                              field2: SafeEnum::Variant3(WithDtor)};
-//~^ ERROR: destructors in statics are an unstable feature
 
 static mut STATIC14: SafeStruct = SafeStruct {
-//~^ ERROR destructors in statics are an unstable feature
     field1: SafeEnum::Variant1,
     field2: SafeEnum::Variant4("str".to_string())
 //~^ ERROR calls in statics are limited to constant functions
@@ -135,7 +111,6 @@ static STATIC16: (&'static Box<MyOwned>, &'static Box<MyOwned>) = (
 );
 
 static mut STATIC17: SafeEnum = SafeEnum::Variant1;
-//~^ ERROR destructors in statics are an unstable feature
 
 static STATIC19: Box<isize> =
     box 3;

src/test/compile-fail/issue-43733-2.rs

@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![feature(const_fn, drop_types_in_const)]
+#![feature(const_fn)]
 #![feature(cfg_target_thread_local, thread_local_internals)]
 
 // On platforms *without* `#[thread_local]`, use

src/test/compile-fail/issue-43733.rs

@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![feature(const_fn, drop_types_in_const)]
+#![feature(const_fn)]
 #![feature(cfg_target_thread_local, thread_local_internals)]
 
 type Foo = std::cell::RefCell<String>;