From 9a649c328c50485de72f057a8003dcbbf3b4dc53 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 12 Oct 2016 14:23:38 +0200 Subject: [PATCH] Require destructors using `#[may_dangle]` to use `unsafe impl`. --- src/librustc/hir/mod.rs | 30 ++++++++++++ src/librustc_typeck/coherence/unsafety.rs | 34 +++++++++----- src/librustc_typeck/diagnostics.rs | 17 +++++++ .../dropck-eyepatch-implies-unsafe-impl.rs | 46 +++++++++++++++++++ 4 files changed, 116 insertions(+), 11 deletions(-) create mode 100644 src/test/compile-fail/dropck-eyepatch-implies-unsafe-impl.rs diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 3ecb731b5ed7..6e81c3e700ed 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -330,6 +330,36 @@ impl Generics { } } +pub enum UnsafeGeneric { + Region(LifetimeDef, &'static str), + Type(TyParam, &'static str), +} + +impl UnsafeGeneric { + pub fn attr_name(&self) -> &'static str { + match *self { + UnsafeGeneric::Region(_, s) => s, + UnsafeGeneric::Type(_, s) => s, + } + } +} + +impl Generics { + pub fn carries_unsafe_attr(&self) -> Option { + for r in &self.lifetimes { + if r.pure_wrt_drop { + return Some(UnsafeGeneric::Region(r.clone(), "may_dangle")); + } + } + for t in &self.ty_params { + if t.pure_wrt_drop { + return Some(UnsafeGeneric::Type(t.clone(), "may_dangle")); + } + } + return None; + } +} + /// A `where` clause in a definition #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)] pub struct WhereClause { diff --git a/src/librustc_typeck/coherence/unsafety.rs b/src/librustc_typeck/coherence/unsafety.rs index ff55ce0e5eb5..cca6c8843067 100644 --- a/src/librustc_typeck/coherence/unsafety.rs +++ b/src/librustc_typeck/coherence/unsafety.rs @@ -13,7 +13,7 @@ use rustc::ty::TyCtxt; use rustc::hir::intravisit; -use rustc::hir; +use rustc::hir::{self, Unsafety}; pub fn check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let mut orphan = UnsafetyChecker { tcx: tcx }; @@ -27,6 +27,7 @@ struct UnsafetyChecker<'cx, 'tcx: 'cx> { impl<'cx, 'tcx, 'v> UnsafetyChecker<'cx, 'tcx> { fn check_unsafety_coherence(&mut self, item: &'v hir::Item, + impl_generics: Option<&hir::Generics>, unsafety: hir::Unsafety, polarity: hir::ImplPolarity) { match self.tcx.impl_trait_ref(self.tcx.map.local_def_id(item.id)) { @@ -47,15 +48,16 @@ impl<'cx, 'tcx, 'v> UnsafetyChecker<'cx, 'tcx> { Some(trait_ref) => { let trait_def = self.tcx.lookup_trait_def(trait_ref.def_id); - match (trait_def.unsafety, unsafety, polarity) { - (hir::Unsafety::Unsafe, hir::Unsafety::Unsafe, hir::ImplPolarity::Negative) => { + let unsafe_attr = impl_generics.and_then(|g| g.carries_unsafe_attr()); + match (trait_def.unsafety, unsafe_attr, unsafety, polarity) { + (_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative) => { span_err!(self.tcx.sess, item.span, E0198, "negative implementations are not unsafe"); } - (hir::Unsafety::Normal, hir::Unsafety::Unsafe, _) => { + (Unsafety::Normal, None, Unsafety::Unsafe, _) => { span_err!(self.tcx.sess, item.span, E0199, @@ -63,7 +65,7 @@ impl<'cx, 'tcx, 'v> UnsafetyChecker<'cx, 'tcx> { trait_ref); } - (hir::Unsafety::Unsafe, hir::Unsafety::Normal, hir::ImplPolarity::Positive) => { + (Unsafety::Unsafe, _, Unsafety::Normal, hir::ImplPolarity::Positive) => { span_err!(self.tcx.sess, item.span, E0200, @@ -71,9 +73,19 @@ impl<'cx, 'tcx, 'v> UnsafetyChecker<'cx, 'tcx> { trait_ref); } - (hir::Unsafety::Unsafe, hir::Unsafety::Normal, hir::ImplPolarity::Negative) | - (hir::Unsafety::Unsafe, hir::Unsafety::Unsafe, hir::ImplPolarity::Positive) | - (hir::Unsafety::Normal, hir::Unsafety::Normal, _) => { + (Unsafety::Normal, Some(g), Unsafety::Normal, hir::ImplPolarity::Positive) => + { + span_err!(self.tcx.sess, + item.span, + E0569, + "requires an `unsafe impl` declaration due to `#[{}]` attribute", + g.attr_name()); + } + + (_, _, Unsafety::Normal, hir::ImplPolarity::Negative) | + (Unsafety::Unsafe, _, Unsafety::Unsafe, hir::ImplPolarity::Positive) | + (Unsafety::Normal, Some(_), Unsafety::Unsafe, hir::ImplPolarity::Positive) | + (Unsafety::Normal, None, Unsafety::Normal, _) => { // OK } } @@ -86,10 +98,10 @@ impl<'cx, 'tcx, 'v> intravisit::Visitor<'v> for UnsafetyChecker<'cx, 'tcx> { fn visit_item(&mut self, item: &'v hir::Item) { match item.node { hir::ItemDefaultImpl(unsafety, _) => { - self.check_unsafety_coherence(item, unsafety, hir::ImplPolarity::Positive); + self.check_unsafety_coherence(item, None, unsafety, hir::ImplPolarity::Positive); } - hir::ItemImpl(unsafety, polarity, ..) => { - self.check_unsafety_coherence(item, unsafety, polarity); + hir::ItemImpl(unsafety, polarity, ref generics, ..) => { + self.check_unsafety_coherence(item, Some(generics), unsafety, polarity); } _ => {} } diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 0d6b43b59c6a..bdd33cb56e0b 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -2819,6 +2819,23 @@ not a distinct static type. Likewise, it's not legal to attempt to behavior for specific enum variants. "##, +E0569: r##" +If an impl has a generic parameter with the `#[may_dangle]` attribute, then +that impl must be declared as an `unsafe impl. For example: + +```compile_fail,E0569 +struct Foo(X); +impl<#[may_dangle] X> Drop for Foo { + fn drop(&mut self) { } +} +``` + +In this example, we are asserting that the destructor for `Foo` will not +access any data of type `X`, and require this assertion to be true for +overall safety in our program. The compiler does not currently attempt to +verify this assertion; therefore we must tag this `impl` as unsafe. +"##, + E0318: r##" Default impls for a trait must be located in the same crate where the trait was defined. For more information see the [opt-in builtin traits RFC](https://github diff --git a/src/test/compile-fail/dropck-eyepatch-implies-unsafe-impl.rs b/src/test/compile-fail/dropck-eyepatch-implies-unsafe-impl.rs new file mode 100644 index 000000000000..f92c8703dc92 --- /dev/null +++ b/src/test/compile-fail/dropck-eyepatch-implies-unsafe-impl.rs @@ -0,0 +1,46 @@ +// Copyright 2016 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(generic_param_attrs)] +#![feature(dropck_eyepatch)] + +// This test ensures that a use of `#[may_dangle]` is rejected if +// it is not attached to an `unsafe impl`. + +use std::fmt; + +struct Dt(&'static str, A); +struct Dr<'a, B:'a+fmt::Debug>(&'static str, &'a B); +struct Pt(&'static str, A, B); +struct Pr<'a, 'b, B:'a+'b+fmt::Debug>(&'static str, &'a B, &'b B); +struct St(&'static str, A); +struct Sr<'a, B:'a+fmt::Debug>(&'static str, &'a B); + +impl Drop for Dt { + fn drop(&mut self) { println!("drop {} {:?}", self.0, self.1); } +} +impl<'a, B: fmt::Debug> Drop for Dr<'a, B> { + fn drop(&mut self) { println!("drop {} {:?}", self.0, self.1); } +} +impl<#[may_dangle] A, B: fmt::Debug> Drop for Pt { + //~^ ERROR requires an `unsafe impl` declaration due to `#[may_dangle]` attribute + + // (unsafe to access self.1 due to #[may_dangle] on A) + fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); } +} +impl<#[may_dangle] 'a, 'b, B: fmt::Debug> Drop for Pr<'a, 'b, B> { + //~^ ERROR requires an `unsafe impl` declaration due to `#[may_dangle]` attribute + + // (unsafe to access self.1 due to #[may_dangle] on 'a) + fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); } +} + +fn main() { +}