Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 5 pull requests #73340

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
95c4899
Added an example where explicitly dropping a lock is necessary/a good…
poliorcetics Jun 7, 2020
9c8f881
Improved the example to work with mutable data, providing a reason fo…
poliorcetics Jun 7, 2020
fdef1a5
Simply use drop instead of std::mem::drop
poliorcetics Jun 8, 2020
1312d30
Remove a lot of unecessary/duplicated comments
poliorcetics Jun 9, 2020
c29b3fa
On recursive ADT, provide indirection structured suggestion
estebank May 29, 2020
7cde07e
review comments: only suggest one substitution
estebank May 31, 2020
03552ec
fix rebase
estebank Jun 10, 2020
c45231c
Revert heterogeneous SocketAddr PartialEq impls
dtolnay Jun 13, 2020
4ff1678
lint: normalize projections using opaque types
davidtwco Jun 12, 2020
f747073
Apply suggestions from code review
poliorcetics Jun 13, 2020
34b3ff0
Clarify the scope-related explanation
poliorcetics Jun 13, 2020
c010e71
Rewrap comments in Mutex example
dtolnay Jun 13, 2020
204c236
Add test for comparing SocketAddr with inferred right-hand side
dtolnay Jun 13, 2020
cecfa43
Move `check_op` logic to `ops` module
ecstatic-morse May 1, 2020
d73674e
Make `Qualifs` getters public
ecstatic-morse May 1, 2020
a77f046
Add feature gate for precise live drop checking
ecstatic-morse Jun 11, 2020
f5370fa
Add `CheckLiveDrops` pass
ecstatic-morse May 3, 2020
a43e486
Add MIR phase and query for drop elaboration
ecstatic-morse May 3, 2020
21ddf4d
Ensure that `drop_elaboration_and_check_consts` runs for all const items
ecstatic-morse May 3, 2020
9e2ee32
Update incorrect error code docs
ecstatic-morse May 3, 2020
2dcf7db
Add tests for `const_precise_live_drops`
ecstatic-morse Jun 11, 2020
1fd1d45
Rollup merge of #71824 - ecstatic-morse:const-check-post-drop-elab, r…
Dylan-DPC Jun 14, 2020
f8b99dd
Rollup merge of #72740 - estebank:recursive-indirection, r=matthewjasper
Dylan-DPC Jun 14, 2020
e1d05a9
Rollup merge of #73104 - poliorcetics:explicit-mutex-drop-example, r=…
Dylan-DPC Jun 14, 2020
30fc113
Rollup merge of #73287 - davidtwco:issue-73251-opaque-types-in-projec…
Dylan-DPC Jun 14, 2020
6b54f86
Rollup merge of #73304 - dtolnay:socketeq, r=Mark-Simulacrum
Dylan-DPC Jun 14, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/librustc_error_codes/error_codes/E0493.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
A type with a `Drop` implementation was destructured when trying to initialize
a static item.
A value with a custom `Drop` implementation may be dropped during const-eval.

Erroneous code example:

Expand All @@ -16,13 +15,14 @@ struct Foo {
field1: DropType,
}

static FOO: Foo = Foo { ..Foo { field1: DropType::A } }; // error!
static FOO: Foo = Foo { field1: (DropType::A, DropType::A).1 }; // error!
```

The problem here is that if the given type or one of its fields implements the
`Drop` trait, this `Drop` implementation cannot be called during the static
type initialization which might cause a memory leak. To prevent this issue,
you need to instantiate all the static type's fields by hand.
`Drop` trait, this `Drop` implementation cannot be called within a const
context since it may run arbitrary, non-const-checked code. To prevent this
issue, ensure all values with custom a custom `Drop` implementation escape the
initializer.

```
enum DropType {
Expand Down
23 changes: 23 additions & 0 deletions src/librustc_errors/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,29 @@ impl Diagnostic {
self
}

pub fn multipart_suggestions(
&mut self,
msg: &str,
suggestions: Vec<Vec<(Span, String)>>,
applicability: Applicability,
) -> &mut Self {
self.suggestions.push(CodeSuggestion {
substitutions: suggestions
.into_iter()
.map(|suggestion| Substitution {
parts: suggestion
.into_iter()
.map(|(span, snippet)| SubstitutionPart { snippet, span })
.collect(),
})
.collect(),
msg: msg.to_owned(),
style: SuggestionStyle::ShowCode,
applicability,
});
self
}

/// Prints out a message with for a multipart suggestion without showing the suggested code.
///
/// This is intended to be used for suggestions that are obvious in what the changes need to
Expand Down
13 changes: 13 additions & 0 deletions src/librustc_errors/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,19 @@ impl<'a> DiagnosticBuilder<'a> {
self
}

pub fn multipart_suggestions(
&mut self,
msg: &str,
suggestions: Vec<Vec<(Span, String)>>,
applicability: Applicability,
) -> &mut Self {
if !self.0.allow_suggestions {
return self;
}
self.0.diagnostic.multipart_suggestions(msg, suggestions, applicability);
self
}

pub fn tool_only_multipart_suggestion(
&mut self,
msg: &str,
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_feature/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,9 @@ declare_features! (
/// Allows `extern "avr-interrupt" fn()` and `extern "avr-non-blocking-interrupt" fn()`.
(active, abi_avr_interrupt, "1.45.0", Some(69664), None),

/// Be more precise when looking for live drops in a const context.
(active, const_precise_live_drops, "1.46.0", Some(73255), None),

// -------------------------------------------------------------------------
// feature-group-end: actual feature gates
// -------------------------------------------------------------------------
Expand Down
6 changes: 5 additions & 1 deletion src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,11 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> {

sess.time("MIR_effect_checking", || {
for def_id in tcx.body_owners() {
mir::transform::check_unsafety::check_unsafety(tcx, def_id)
mir::transform::check_unsafety::check_unsafety(tcx, def_id);

if tcx.hir().body_const_context(def_id).is_some() {
tcx.ensure().mir_drops_elaborated_and_const_checked(def_id);
}
}
});

Expand Down
27 changes: 19 additions & 8 deletions src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,22 +927,33 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool {
use rustc_middle::ty::TypeFoldable;

struct ProhibitOpaqueTypes<'tcx> {
struct ProhibitOpaqueTypes<'a, 'tcx> {
cx: &'a LateContext<'a, 'tcx>,
ty: Option<Ty<'tcx>>,
};

impl<'tcx> ty::fold::TypeVisitor<'tcx> for ProhibitOpaqueTypes<'tcx> {
impl<'a, 'tcx> ty::fold::TypeVisitor<'tcx> for ProhibitOpaqueTypes<'a, 'tcx> {
fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
if let ty::Opaque(..) = ty.kind {
self.ty = Some(ty);
true
} else {
ty.super_visit_with(self)
match ty.kind {
ty::Opaque(..) => {
self.ty = Some(ty);
true
}
// Consider opaque types within projections FFI-safe if they do not normalize
// to more opaque types.
ty::Projection(..) => {
let ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, ty);

// If `ty` is a opaque type directly then `super_visit_with` won't invoke
// this function again.
if ty.has_opaque_types() { self.visit_ty(ty) } else { false }
}
_ => ty.super_visit_with(self),
}
}
}

let mut visitor = ProhibitOpaqueTypes { ty: None };
let mut visitor = ProhibitOpaqueTypes { cx: self.cx, ty: None };
ty.visit_with(&mut visitor);
if let Some(ty) = visitor.ty {
self.emit_ffi_unsafe_type_lint(ty, sp, "opaque types have no C equivalent", None);
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_middle/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ pub enum MirPhase {
Build = 0,
Const = 1,
Validated = 2,
Optimized = 3,
DropElab = 3,
Optimized = 4,
}

impl MirPhase {
Expand Down
6 changes: 6 additions & 0 deletions src/librustc_middle/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,12 @@ rustc_queries! {
no_hash
}

query mir_drops_elaborated_and_const_checked(key: LocalDefId) -> Steal<mir::Body<'tcx>> {
storage(ArenaCacheSelector<'tcx>)
no_hash
desc { |tcx| "elaborating drops for `{}`", tcx.def_path_str(key.to_def_id()) }
}

query mir_validated(key: LocalDefId) ->
(
Steal<mir::Body<'tcx>>,
Expand Down
10 changes: 9 additions & 1 deletion src/librustc_middle/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,15 @@ impl<'tcx> ty::TyS<'tcx> {
// Find non representable fields with their spans
fold_repr(def.all_fields().map(|field| {
let ty = field.ty(tcx, substs);
let span = tcx.hir().span_if_local(field.did).unwrap_or(sp);
let span = match field
.did
.as_local()
.map(|id| tcx.hir().as_local_hir_id(id))
.and_then(|id| tcx.hir().find(id))
{
Some(hir::Node::Field(field)) => field.ty.span,
_ => sp,
};
match is_type_structurally_recursive(
tcx,
span,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/transform/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use rustc_middle::ty::{self, TyCtxt};
pub use self::qualifs::Qualif;

mod ops;
pub mod post_drop_elaboration;
pub mod qualifs;
mod resolver;
pub mod validation;
Expand Down
16 changes: 16 additions & 0 deletions src/librustc_mir/transform/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,22 @@ use rustc_span::{Span, Symbol};

use super::ConstCx;

/// Emits an error if `op` is not allowed in the given const context.
pub fn non_const<O: NonConstOp>(ccx: &ConstCx<'_, '_>, op: O, span: Span) {
debug!("illegal_op: op={:?}", op);

if op.is_allowed_in_item(ccx) {
return;
}

if ccx.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
ccx.tcx.sess.miri_unleashed_feature(span, O::feature_gate());
return;
}

op.emit_error(ccx, span);
}

/// An operation that is not *always* allowed in a const context.
pub trait NonConstOp: std::fmt::Debug {
/// Returns the `Symbol` corresponding to the feature gate that would enable this operation,
Expand Down
119 changes: 119 additions & 0 deletions src/librustc_mir/transform/check_consts/post_drop_elaboration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
use rustc_hir::def_id::LocalDefId;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{self, BasicBlock, Location};
use rustc_middle::ty::TyCtxt;
use rustc_span::Span;

use super::ops;
use super::qualifs::{NeedsDrop, Qualif};
use super::validation::Qualifs;
use super::ConstCx;

/// Returns `true` if we should use the more precise live drop checker that runs after drop
/// elaboration.
pub fn checking_enabled(tcx: TyCtxt<'tcx>) -> bool {
tcx.features().const_precise_live_drops
}

/// Look for live drops in a const context.
///
/// This is separate from the rest of the const checking logic because it must run after drop
/// elaboration.
pub fn check_live_drops(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &mir::Body<'tcx>) {
let const_kind = tcx.hir().body_const_context(def_id);
if const_kind.is_none() {
return;
}

if !checking_enabled(tcx) {
return;
}

let ccx = ConstCx {
body,
tcx,
def_id: def_id.to_def_id(),
const_kind,
param_env: tcx.param_env(def_id),
};

let mut visitor = CheckLiveDrops { ccx: &ccx, qualifs: Qualifs::default() };

visitor.visit_body(body);
}

struct CheckLiveDrops<'mir, 'tcx> {
ccx: &'mir ConstCx<'mir, 'tcx>,
qualifs: Qualifs<'mir, 'tcx>,
}

// So we can access `body` and `tcx`.
impl std::ops::Deref for CheckLiveDrops<'mir, 'tcx> {
type Target = ConstCx<'mir, 'tcx>;

fn deref(&self) -> &Self::Target {
&self.ccx
}
}

impl CheckLiveDrops<'mir, 'tcx> {
fn check_live_drop(&self, span: Span) {
ops::non_const(self.ccx, ops::LiveDrop, span);
}
}

impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
fn visit_basic_block_data(&mut self, bb: BasicBlock, block: &mir::BasicBlockData<'tcx>) {
trace!("visit_basic_block_data: bb={:?} is_cleanup={:?}", bb, block.is_cleanup);

// Ignore drop terminators in cleanup blocks.
if block.is_cleanup {
return;
}

self.super_basic_block_data(bb, block);
}

fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) {
trace!("visit_terminator: terminator={:?} location={:?}", terminator, location);

match &terminator.kind {
mir::TerminatorKind::Drop { location: dropped_place, .. } => {
let dropped_ty = dropped_place.ty(self.body, self.tcx).ty;
if !NeedsDrop::in_any_value_of_ty(self.ccx, dropped_ty) {
return;
}

if dropped_place.is_indirect() {
self.check_live_drop(terminator.source_info.span);
return;
}

if self.qualifs.needs_drop(self.ccx, dropped_place.local, location) {
// Use the span where the dropped local was declared for the error.
let span = self.body.local_decls[dropped_place.local].source_info.span;
self.check_live_drop(span);
}
}

mir::TerminatorKind::DropAndReplace { .. } => span_bug!(
terminator.source_info.span,
"`DropAndReplace` should be removed by drop elaboration",
),

mir::TerminatorKind::Abort
| mir::TerminatorKind::Call { .. }
| mir::TerminatorKind::Assert { .. }
| mir::TerminatorKind::FalseEdge { .. }
| mir::TerminatorKind::FalseUnwind { .. }
| mir::TerminatorKind::GeneratorDrop
| mir::TerminatorKind::Goto { .. }
| mir::TerminatorKind::InlineAsm { .. }
| mir::TerminatorKind::Resume
| mir::TerminatorKind::Return
| mir::TerminatorKind::SwitchInt { .. }
| mir::TerminatorKind::Unreachable
| mir::TerminatorKind::Yield { .. } => {}
}
}
}
Loading