Skip to content

Commit

Permalink
Auto merge of #85437 - GuillaumeGomez:rollup-3jcirty, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
Rollup of 7 pull requests

Successful merges:

 - #84587 (rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint (`INVALID_RUST_CODEBLOCKS`))
 - #85280 (Toggle-wrap items differently than top-doc.)
 - #85338 (Implement more Iterator methods on core::iter::Repeat)
 - #85339 (Report an error if a lang item has the wrong number of generic arguments)
 - #85369 (Suggest borrowing if a trait implementation is found for &/&mut <type>)
 - #85393 (Suppress spurious errors inside `async fn`)
 - #85415 (Clean up remnants of BorrowOfPackedField)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed May 18, 2021
2 parents 70b9d1e + f4a0d97 commit a5560a6
Show file tree
Hide file tree
Showing 42 changed files with 800 additions and 133 deletions.
6 changes: 0 additions & 6 deletions compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ pub enum UnsafetyViolationDetails {
UseOfInlineAssembly,
InitializingTypeWith,
CastOfPointerToInt,
BorrowOfPackedField,
UseOfMutableStatic,
UseOfExternStatic,
DerefOfRawPointer,
Expand Down Expand Up @@ -64,11 +63,6 @@ impl UnsafetyViolationDetails {
CastOfPointerToInt => {
("cast of pointer to int", "casting pointers to integers in constants")
}
BorrowOfPackedField => (
"borrow of packed field",
"fields of packed structs might be misaligned: dereferencing a misaligned pointer \
or even just creating a misaligned reference is undefined behavior",
),
UseOfMutableStatic => (
"use of mutable static",
"mutable statics can be mutated by multiple threads: aliasing violations or data \
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/borrow_check/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// it. However, it works pretty well in practice. In particular,
/// this is needed to deal with projection outlives bounds like
///
/// ```ignore (internal compiler representation so lifetime syntax is invalid)
/// ```text
/// <T as Foo<'0>>::Item: '1
/// ```
///
Expand Down
59 changes: 22 additions & 37 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,38 +64,30 @@ impl<'tcx> UnsafetyVisitor<'tcx> {
SafetyContext::UnsafeFn if unsafe_op_in_unsafe_fn_allowed => {}
SafetyContext::UnsafeFn => {
// unsafe_op_in_unsafe_fn is disallowed
if kind == BorrowOfPackedField {
// FIXME handle borrows of packed fields
} else {
struct_span_err!(
self.tcx.sess,
span,
E0133,
"{} is unsafe and requires unsafe block",
description,
)
.span_label(span, description)
.note(note)
.emit();
}
struct_span_err!(
self.tcx.sess,
span,
E0133,
"{} is unsafe and requires unsafe block",
description,
)
.span_label(span, description)
.note(note)
.emit();
}
SafetyContext::Safe => {
if kind == BorrowOfPackedField {
// FIXME handle borrows of packed fields
} else {
let fn_sugg = if unsafe_op_in_unsafe_fn_allowed { " function or" } else { "" };
struct_span_err!(
self.tcx.sess,
span,
E0133,
"{} is unsafe and requires unsafe{} block",
description,
fn_sugg,
)
.span_label(span, description)
.note(note)
.emit();
}
let fn_sugg = if unsafe_op_in_unsafe_fn_allowed { " function or" } else { "" };
struct_span_err!(
self.tcx.sess,
span,
E0133,
"{} is unsafe and requires unsafe{} block",
description,
fn_sugg,
)
.span_label(span, description)
.note(note)
.emit();
}
}
}
Expand Down Expand Up @@ -203,8 +195,6 @@ enum UnsafeOpKind {
#[allow(dead_code)] // FIXME
CastOfPointerToInt,
#[allow(dead_code)] // FIXME
BorrowOfPackedField,
#[allow(dead_code)] // FIXME
UseOfMutableStatic,
#[allow(dead_code)] // FIXME
UseOfExternStatic,
Expand Down Expand Up @@ -244,11 +234,6 @@ impl UnsafeOpKind {
CastOfPointerToInt => {
("cast of pointer to int", "casting pointers to integers in constants")
}
BorrowOfPackedField => (
"borrow of packed field",
"fields of packed structs might be misaligned: dereferencing a misaligned pointer \
or even just creating a misaligned reference is undefined behavior",
),
UseOfMutableStatic => (
"use of mutable static",
"mutable statics can be mutated by multiple threads: aliasing violations or data \
Expand Down
127 changes: 124 additions & 3 deletions compiler/rustc_passes/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ use crate::weak_lang_items;
use rustc_middle::middle::cstore::ExternCrate;
use rustc_middle::ty::TyCtxt;

use rustc_errors::struct_span_err;
use rustc_errors::{pluralize, struct_span_err};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::itemlikevisit::ItemLikeVisitor;
use rustc_hir::lang_items::{extract, ITEM_REFS};
use rustc_hir::{HirId, LangItem, LanguageItems, Target};
use rustc_span::Span;

use rustc_middle::ty::query::Providers;

Expand Down Expand Up @@ -61,8 +62,7 @@ impl LanguageItemCollector<'tcx> {
match ITEM_REFS.get(&value).cloned() {
// Known lang item with attribute on correct target.
Some((item_index, expected_target)) if actual_target == expected_target => {
let def_id = self.tcx.hir().local_def_id(hir_id);
self.collect_item(item_index, def_id.to_def_id());
self.collect_item_extended(item_index, hir_id, span);
}
// Known lang item with attribute on incorrect target.
Some((_, expected_target)) => {
Expand Down Expand Up @@ -180,6 +180,127 @@ impl LanguageItemCollector<'tcx> {
self.items.groups[group as usize].push(item_def_id);
}
}

// Like collect_item() above, but also checks whether the lang item is declared
// with the right number of generic arguments if it is a trait.
fn collect_item_extended(&mut self, item_index: usize, hir_id: HirId, span: Span) {
let item_def_id = self.tcx.hir().local_def_id(hir_id).to_def_id();
let lang_item = LangItem::from_u32(item_index as u32).unwrap();
let name = lang_item.name();

self.collect_item(item_index, item_def_id);

// Now check whether the lang_item has the expected number of generic
// arguments if it is a trait. Generally speaking, binary and indexing
// operations have one (for the RHS/index), unary operations have none,
// and the rest also have none except for the closure traits (one for
// the argument list), generators (one for the resume argument),
// ordering/equality relations (one for the RHS), and various conversion
// traits.

let expected_num = match lang_item {
// Binary operations
LangItem::Add
| LangItem::Sub
| LangItem::Mul
| LangItem::Div
| LangItem::Rem
| LangItem::BitXor
| LangItem::BitAnd
| LangItem::BitOr
| LangItem::Shl
| LangItem::Shr
| LangItem::AddAssign
| LangItem::SubAssign
| LangItem::MulAssign
| LangItem::DivAssign
| LangItem::RemAssign
| LangItem::BitXorAssign
| LangItem::BitAndAssign
| LangItem::BitOrAssign
| LangItem::ShlAssign
| LangItem::ShrAssign
| LangItem::Index
| LangItem::IndexMut

// Miscellaneous
| LangItem::Unsize
| LangItem::CoerceUnsized
| LangItem::DispatchFromDyn
| LangItem::Fn
| LangItem::FnMut
| LangItem::FnOnce
| LangItem::Generator
| LangItem::PartialEq
| LangItem::PartialOrd
=> Some(1),

// Unary operations
LangItem::Neg
| LangItem::Not

// Miscellaneous
| LangItem::Deref
| LangItem::DerefMut
| LangItem::Sized
| LangItem::StructuralPeq
| LangItem::StructuralTeq
| LangItem::Copy
| LangItem::Clone
| LangItem::Sync
| LangItem::DiscriminantKind
| LangItem::PointeeTrait
| LangItem::Freeze
| LangItem::Drop
| LangItem::Receiver
| LangItem::Future
| LangItem::Unpin
| LangItem::Termination
| LangItem::Try
| LangItem::Send
| LangItem::UnwindSafe
| LangItem::RefUnwindSafe
=> Some(0),

// Not a trait
_ => None,
};

if let Some(expected_num) = expected_num {
let (actual_num, generics_span) = match self.tcx.hir().get(hir_id) {
hir::Node::Item(hir::Item {
kind: hir::ItemKind::Trait(_, _, generics, ..),
..
}) => (generics.params.len(), generics.span),
_ => bug!("op/index/deref lang item target is not a trait: {:?}", lang_item),
};

if expected_num != actual_num {
// We are issuing E0718 "incorrect target" here, because while the
// item kind of the target is correct, the target is still wrong
// because of the wrong number of generic arguments.
struct_span_err!(
self.tcx.sess,
span,
E0718,
"`{}` language item must be applied to a trait with {} generic argument{}",
name,
expected_num,
pluralize!(expected_num)
)
.span_label(
generics_span,
format!(
"this trait has {} generic argument{}, not {}",
actual_num,
pluralize!(actual_num),
expected_num
),
)
.emit();
}
}
}
}

/// Traverses and collects all the lang items in all crates.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/src/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ pub struct OpaqueTypeDecl<'tcx> {
/// type Foo = impl Baz;
/// fn bar() -> Foo {
/// // ^^^ This is the span we are looking for!
/// }
/// ```
///
/// In cases where the fn returns `(impl Trait, impl Trait)` or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -686,17 +686,36 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
return false;
}

// Blacklist traits for which it would be nonsensical to suggest borrowing.
// For instance, immutable references are always Copy, so suggesting to
// borrow would always succeed, but it's probably not what the user wanted.
let blacklist: Vec<_> =
[LangItem::Copy, LangItem::Clone, LangItem::Unpin, LangItem::Sized, LangItem::Send]
.iter()
.filter_map(|lang_item| self.tcx.lang_items().require(*lang_item).ok())
.collect();

let span = obligation.cause.span;
let param_env = obligation.param_env;
let trait_ref = trait_ref.skip_binder();

if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code {
// Try to apply the original trait binding obligation by borrowing.
let self_ty = trait_ref.self_ty();
let found = self_ty.to_string();
let new_self_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, self_ty);
let substs = self.tcx.mk_substs_trait(new_self_ty, &[]);
let new_trait_ref = ty::TraitRef::new(obligation.parent_trait_ref.def_id(), substs);
let found_ty = trait_ref.self_ty();
let found_ty_str = found_ty.to_string();
let imm_borrowed_found_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, found_ty);
let imm_substs = self.tcx.mk_substs_trait(imm_borrowed_found_ty, &[]);
let mut_borrowed_found_ty = self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, found_ty);
let mut_substs = self.tcx.mk_substs_trait(mut_borrowed_found_ty, &[]);

// Try to apply the original trait binding obligation by borrowing.
let mut try_borrowing = |new_trait_ref: ty::TraitRef<'tcx>,
expected_trait_ref: ty::TraitRef<'tcx>,
mtbl: bool,
blacklist: &[DefId]|
-> bool {
if blacklist.contains(&expected_trait_ref.def_id) {
return false;
}

let new_obligation = Obligation::new(
ObligationCause::dummy(),
param_env,
Expand All @@ -713,8 +732,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {

let msg = format!(
"the trait bound `{}: {}` is not satisfied",
found,
obligation.parent_trait_ref.skip_binder().print_only_trait_path(),
found_ty_str,
expected_trait_ref.print_only_trait_path(),
);
if has_custom_message {
err.note(&msg);
Expand All @@ -730,7 +749,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
span,
&format!(
"expected an implementor of trait `{}`",
obligation.parent_trait_ref.skip_binder().print_only_trait_path(),
expected_trait_ref.print_only_trait_path(),
),
);

Expand All @@ -745,16 +764,52 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {

err.span_suggestion(
span,
"consider borrowing here",
format!("&{}", snippet),
&format!(
"consider{} borrowing here",
if mtbl { " mutably" } else { "" }
),
format!("&{}{}", if mtbl { "mut " } else { "" }, snippet),
Applicability::MaybeIncorrect,
);
}
return true;
}
}
return false;
};

if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code {
let expected_trait_ref = obligation.parent_trait_ref.skip_binder();
let new_imm_trait_ref =
ty::TraitRef::new(obligation.parent_trait_ref.def_id(), imm_substs);
let new_mut_trait_ref =
ty::TraitRef::new(obligation.parent_trait_ref.def_id(), mut_substs);
if try_borrowing(new_imm_trait_ref, expected_trait_ref, false, &[]) {
return true;
} else {
return try_borrowing(new_mut_trait_ref, expected_trait_ref, true, &[]);
}
} else if let ObligationCauseCode::BindingObligation(_, _)
| ObligationCauseCode::ItemObligation(_) = &obligation.cause.code
{
if try_borrowing(
ty::TraitRef::new(trait_ref.def_id, imm_substs),
trait_ref,
false,
&blacklist[..],
) {
return true;
} else {
return try_borrowing(
ty::TraitRef::new(trait_ref.def_id, mut_substs),
trait_ref,
true,
&blacklist[..],
);
}
} else {
false
}
false
}

/// Whenever references are used by mistake, like `for (i, e) in &vec.iter().enumerate()`,
Expand Down
Loading

0 comments on commit a5560a6

Please sign in to comment.