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 6 pull requests #102652

Merged
merged 19 commits into from
Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
5ed1787
Implement `Ready::into_inner()`
daxpedda Aug 30, 2022
fa61678
Fix leaking in inplace collection when destructor panics
SkiFire13 Sep 10, 2022
f81b07e
Adapt inplace collection leak test to check for no leaks
SkiFire13 Sep 10, 2022
f52082f
Update documentation
SkiFire13 Sep 10, 2022
dad049c
Update test
SkiFire13 Sep 10, 2022
c7d1ec0
Don't ICE when trying to copy unsized value in const prop
compiler-errors Oct 2, 2022
d0d6af9
Lint for unsatisfied nested opaques
compiler-errors Oct 2, 2022
426424b
Make it a lint for all opaque types
compiler-errors Oct 2, 2022
7a88540
Add example to opaque_hidden_inferred_bound lint
compiler-errors Oct 2, 2022
1750c7b
Clarify documentation
SkiFire13 Sep 10, 2022
1456f73
Fix rustdoc ICE in invalid_rust_codeblocks lint
Noratrieb Oct 3, 2022
8c60012
Normalize substs before resolving instance in NoopMethodCall lint
compiler-errors Sep 29, 2022
e1b313a
We are able to resolve methods even if they need subst
compiler-errors Oct 4, 2022
c1d4003
Rollup merge of #101189 - daxpedda:ready-into-inner, r=joshtriplett
Dylan-DPC Oct 4, 2022
f24d00d
Rollup merge of #101642 - SkiFire13:fix-inplace-collection-leak, r=th…
Dylan-DPC Oct 4, 2022
d89d214
Rollup merge of #102489 - compiler-errors:issue-102074, r=oli-obk
Dylan-DPC Oct 4, 2022
32dde23
Rollup merge of #102559 - compiler-errors:issue-102553, r=oli-obk
Dylan-DPC Oct 4, 2022
35f92ed
Rollup merge of #102568 - compiler-errors:lint-unsatisfied-opaques, r…
Dylan-DPC Oct 4, 2022
f7ca465
Rollup merge of #102633 - Nilstrieb:rustdoc-lint-🏳️‍⚧️late, r=davidtwco
Dylan-DPC Oct 4, 2022
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
16 changes: 11 additions & 5 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,11 +640,17 @@ where
// avoid force_allocation.
let src = match self.read_immediate_raw(src)? {
Ok(src_val) => {
assert!(!src.layout.is_unsized(), "cannot copy unsized immediates");
assert!(
!dest.layout.is_unsized(),
"the src is sized, so the dest must also be sized"
);
// FIXME(const_prop): Const-prop can possibly evaluate an
// unsized copy operation when it thinks that the type is
// actually sized, due to a trivially false where-clause
// predicate like `where Self: Sized` with `Self = dyn Trait`.
// See #102553 for an example of such a predicate.
if src.layout.is_unsized() {
throw_inval!(SizeOfUnsizedType(src.layout.ty));
}
if dest.layout.is_unsized() {
throw_inval!(SizeOfUnsizedType(dest.layout.ty));
}
assert_eq!(src.layout.size, dest.layout.size);
// Yay, we got a value that we can write directly.
return if layout_compat {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_error_messages/locales/en-US/lint.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -433,3 +433,7 @@ lint_check_name_unknown_tool = unknown lint tool: `{$tool_name}`
lint_check_name_warning = {$msg}

lint_check_name_deprecated = lint name `{$lint_name}` is deprecated and does not have an effect anymore. Use: {$new_name}

lint_opaque_hidden_inferred_bound = opaque type `{$ty}` does not satisfy its associated type bounds
.specifically = this associated type bound is unsatisfied for `{$proj_ty}`
.suggestion = add this bound
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ mod non_ascii_idents;
mod non_fmt_panic;
mod nonstandard_style;
mod noop_method_call;
mod opaque_hidden_inferred_bound;
mod pass_by_value;
mod passes;
mod redundant_semicolon;
Expand Down Expand Up @@ -93,6 +94,7 @@ use non_ascii_idents::*;
use non_fmt_panic::NonPanicFmt;
use nonstandard_style::*;
use noop_method_call::*;
use opaque_hidden_inferred_bound::*;
use pass_by_value::*;
use redundant_semicolon::*;
use traits::*;
Expand Down Expand Up @@ -223,6 +225,7 @@ macro_rules! late_lint_mod_passes {
EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums,
InvalidAtomicOrdering: InvalidAtomicOrdering,
NamedAsmLabels: NamedAsmLabels,
OpaqueHiddenInferredBound: OpaqueHiddenInferredBound,
]
);
};
Expand Down
17 changes: 6 additions & 11 deletions compiler/rustc_lint/src/noop_method_call.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::context::LintContext;
use crate::rustc_middle::ty::TypeVisitable;
use crate::LateContext;
use crate::LateLintPass;
use rustc_errors::fluent;
Expand Down Expand Up @@ -46,7 +45,7 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
};
// We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow`
// traits and ignore any other method call.
let (trait_id, did) = match cx.typeck_results().type_dependent_def(expr.hir_id) {
let did = match cx.typeck_results().type_dependent_def(expr.hir_id) {
// Verify we are dealing with a method/associated function.
Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) {
// Check that we're dealing with a trait method for one of the traits we care about.
Expand All @@ -56,21 +55,17 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
Some(sym::Borrow | sym::Clone | sym::Deref)
) =>
{
(trait_id, did)
did
}
_ => return,
},
_ => return,
};
let substs = cx.typeck_results().node_substs(expr.hir_id);
if substs.needs_subst() {
// We can't resolve on types that require monomorphization, so we don't handle them if
// we need to perform substitution.
return;
}
let param_env = cx.tcx.param_env(trait_id);
let substs = cx
.tcx
.normalize_erasing_regions(cx.param_env, cx.typeck_results().node_substs(expr.hir_id));
// Resolve the trait method instance.
let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) else {
let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, cx.param_env, did, substs) else {
return
};
// (Re)check that it implements the noop diagnostic.
Expand Down
156 changes: 156 additions & 0 deletions compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
use rustc_hir as hir;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_macros::LintDiagnostic;
use rustc_middle::ty::{self, fold::BottomUpFolder, Ty, TypeFoldable};
use rustc_span::Span;
use rustc_trait_selection::traits;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;

use crate::{LateContext, LateLintPass, LintContext};

declare_lint! {
/// The `opaque_hidden_inferred_bound` lint detects cases in which nested
/// `impl Trait` in associated type bounds are not written generally enough
/// to satisfy the bounds of the associated type.
///
/// ### Explanation
///
/// This functionality was removed in #97346, but then rolled back in #99860
/// because it caused regressions.
///
/// We plan on reintroducing this as a hard error, but in the mean time,
/// this lint serves to warn and suggest fixes for any use-cases which rely
/// on this behavior.
///
/// ### Example
///
/// ```
/// trait Trait {
/// type Assoc: Send;
/// }
///
/// struct Struct;
///
/// impl Trait for Struct {
/// type Assoc = i32;
/// }
///
/// fn test() -> impl Trait<Assoc = impl Sized> {
/// Struct
/// }
/// ```
///
/// {{produces}}
///
/// In this example, `test` declares that the associated type `Assoc` for
/// `impl Trait` is `impl Sized`, which does not satisfy the `Send` bound
/// on the associated type.
///
/// Although the hidden type, `i32` does satisfy this bound, we do not
/// consider the return type to be well-formed with this lint. It can be
/// fixed by changing `impl Sized` into `impl Sized + Send`.
pub OPAQUE_HIDDEN_INFERRED_BOUND,
Warn,
"detects the use of nested `impl Trait` types in associated type bounds that are not general enough"
}

declare_lint_pass!(OpaqueHiddenInferredBound => [OPAQUE_HIDDEN_INFERRED_BOUND]);

impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
let hir::ItemKind::OpaqueTy(_) = &item.kind else { return; };
let def_id = item.def_id.def_id.to_def_id();
cx.tcx.infer_ctxt().enter(|ref infcx| {
// For every projection predicate in the opaque type's explicit bounds,
// check that the type that we're assigning actually satisfies the bounds
// of the associated type.
for &(pred, pred_span) in cx.tcx.explicit_item_bounds(def_id) {
// Liberate bound regions in the predicate since we
// don't actually care about lifetimes in this check.
let predicate = cx.tcx.liberate_late_bound_regions(
def_id,
pred.kind(),
);
let ty::PredicateKind::Projection(proj) = predicate else {
continue;
};
// Only check types, since those are the only things that may
// have opaques in them anyways.
let Some(proj_term) = proj.term.ty() else { continue };

let proj_ty =
cx
.tcx
.mk_projection(proj.projection_ty.item_def_id, proj.projection_ty.substs);
// For every instance of the projection type in the bounds,
// replace them with the term we're assigning to the associated
// type in our opaque type.
let proj_replacer = &mut BottomUpFolder {
tcx: cx.tcx,
ty_op: |ty| if ty == proj_ty { proj_term } else { ty },
lt_op: |lt| lt,
ct_op: |ct| ct,
};
// For example, in `impl Trait<Assoc = impl Send>`, for all of the bounds on `Assoc`,
// e.g. `type Assoc: OtherTrait`, replace `<impl Trait as Trait>::Assoc: OtherTrait`
// with `impl Send: OtherTrait`.
for assoc_pred_and_span in cx
.tcx
.bound_explicit_item_bounds(proj.projection_ty.item_def_id)
.transpose_iter()
{
let assoc_pred_span = assoc_pred_and_span.0.1;
let assoc_pred = assoc_pred_and_span
.map_bound(|(pred, _)| *pred)
.subst(cx.tcx, &proj.projection_ty.substs)
.fold_with(proj_replacer);
let Ok(assoc_pred) = traits::fully_normalize(infcx, traits::ObligationCause::dummy(), cx.param_env, assoc_pred) else {
continue;
};
// If that predicate doesn't hold modulo regions (but passed during type-check),
// then we must've taken advantage of the hack in `project_and_unify_types` where
// we replace opaques with inference vars. Emit a warning!
if !infcx.predicate_must_hold_modulo_regions(&traits::Obligation::new(
traits::ObligationCause::dummy(),
cx.param_env,
assoc_pred,
)) {
// If it's a trait bound and an opaque that doesn't satisfy it,
// then we can emit a suggestion to add the bound.
let (suggestion, suggest_span) =
match (proj_term.kind(), assoc_pred.kind().skip_binder()) {
(ty::Opaque(def_id, _), ty::PredicateKind::Trait(trait_pred)) => (
format!(" + {}", trait_pred.print_modifiers_and_trait_path()),
Some(cx.tcx.def_span(def_id).shrink_to_hi()),
),
_ => (String::new(), None),
};
cx.emit_spanned_lint(
OPAQUE_HIDDEN_INFERRED_BOUND,
pred_span,
OpaqueHiddenInferredBoundLint {
ty: cx.tcx.mk_opaque(def_id, ty::InternalSubsts::identity_for_item(cx.tcx, def_id)),
proj_ty: proj_term,
assoc_pred_span,
suggestion,
suggest_span,
},
);
}
}
}
});
}
}

#[derive(LintDiagnostic)]
#[diag(lint::opaque_hidden_inferred_bound)]
struct OpaqueHiddenInferredBoundLint<'tcx> {
ty: Ty<'tcx>,
proj_ty: Ty<'tcx>,
#[label(lint::specifically)]
assoc_pred_span: Span,
#[suggestion_verbose(applicability = "machine-applicable", code = "{suggestion}")]
suggest_span: Option<Span>,
suggestion: String,
}
4 changes: 3 additions & 1 deletion compiler/rustc_middle/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,9 @@ pub fn in_external_macro(sess: &Session, span: Span) -> bool {
match expn_data.kind {
ExpnKind::Inlined
| ExpnKind::Root
| ExpnKind::Desugaring(DesugaringKind::ForLoop | DesugaringKind::WhileLoop) => false,
| ExpnKind::Desugaring(
DesugaringKind::ForLoop | DesugaringKind::WhileLoop | DesugaringKind::OpaqueTy,
) => false,
ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) => true, // well, it's "external"
ExpnKind::Macro(MacroKind::Bang, _) => {
// Dummy span for the `def_site` means it's an external macro.
Expand Down
14 changes: 10 additions & 4 deletions library/alloc/src/vec/in_place_collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@
//! This is handled by the [`InPlaceDrop`] guard for sink items (`U`) and by
//! [`vec::IntoIter::forget_allocation_drop_remaining()`] for remaining source items (`T`).
//!
//! If dropping any remaining source item (`T`) panics then [`InPlaceDstBufDrop`] will handle dropping
//! the already collected sink items (`U`) and freeing the allocation.
//!
//! [`vec::IntoIter::forget_allocation_drop_remaining()`]: super::IntoIter::forget_allocation_drop_remaining()
//!
//! # O(1) collect
Expand Down Expand Up @@ -138,7 +141,7 @@ use core::iter::{InPlaceIterable, SourceIter, TrustedRandomAccessNoCoerce};
use core::mem::{self, ManuallyDrop, SizedTypeProperties};
use core::ptr::{self};

use super::{InPlaceDrop, SpecFromIter, SpecFromIterNested, Vec};
use super::{InPlaceDrop, InPlaceDstBufDrop, SpecFromIter, SpecFromIterNested, Vec};

/// Specialization marker for collecting an iterator pipeline into a Vec while reusing the
/// source allocation, i.e. executing the pipeline in place.
Expand Down Expand Up @@ -191,14 +194,17 @@ where
);
}

// Drop any remaining values at the tail of the source but prevent drop of the allocation
// itself once IntoIter goes out of scope.
// If the drop panics then we also leak any elements collected into dst_buf.
// The ownership of the allocation and the new `T` values is temporarily moved into `dst_guard`.
// This is safe because `forget_allocation_drop_remaining` immediately forgets the allocation
// before any panic can occur in order to avoid any double free, and then proceeds to drop
// any remaining values at the tail of the source.
//
// Note: This access to the source wouldn't be allowed by the TrustedRandomIteratorNoCoerce
// contract (used by SpecInPlaceCollect below). But see the "O(1) collect" section in the
// module documenttation why this is ok anyway.
let dst_guard = InPlaceDstBufDrop { ptr: dst_buf, len, cap };
src.forget_allocation_drop_remaining();
mem::forget(dst_guard);

let vec = unsafe { Vec::from_raw_parts(dst_buf, len, cap) };

Expand Down
15 changes: 15 additions & 0 deletions library/alloc/src/vec/in_place_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,18 @@ impl<T> Drop for InPlaceDrop<T> {
}
}
}

// A helper struct for in-place collection that drops the destination allocation and elements,
// to avoid leaking them if some other destructor panics.
pub(super) struct InPlaceDstBufDrop<T> {
pub(super) ptr: *mut T,
pub(super) len: usize,
pub(super) cap: usize,
}

impl<T> Drop for InPlaceDstBufDrop<T> {
#[inline]
fn drop(&mut self) {
unsafe { super::Vec::from_raw_parts(self.ptr, self.len, self.cap) };
}
}
7 changes: 6 additions & 1 deletion library/alloc/src/vec/into_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,16 @@ impl<T, A: Allocator> IntoIter<T, A> {
}

/// Drops remaining elements and relinquishes the backing allocation.
/// This method guarantees it won't panic before relinquishing
/// the backing allocation.
///
/// This is roughly equivalent to the following, but more efficient
///
/// ```
/// # let mut into_iter = Vec::<u8>::with_capacity(10).into_iter();
/// let mut into_iter = std::mem::replace(&mut into_iter, Vec::new().into_iter());
/// (&mut into_iter).for_each(core::mem::drop);
/// unsafe { core::ptr::write(&mut into_iter, Vec::new().into_iter()); }
/// std::mem::forget(into_iter);
/// ```
///
/// This method is used by in-place iteration, refer to the vec::in_place_collect
Expand All @@ -118,6 +121,8 @@ impl<T, A: Allocator> IntoIter<T, A> {
self.ptr = self.buf.as_ptr();
self.end = self.buf.as_ptr();

// Dropping the remaining elements can panic, so this needs to be
// done only after updating the other fields.
unsafe {
ptr::drop_in_place(remaining);
}
Expand Down
2 changes: 1 addition & 1 deletion library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ use self::set_len_on_drop::SetLenOnDrop;
mod set_len_on_drop;

#[cfg(not(no_global_oom_handling))]
use self::in_place_drop::InPlaceDrop;
use self::in_place_drop::{InPlaceDrop, InPlaceDstBufDrop};

#[cfg(not(no_global_oom_handling))]
mod in_place_drop;
Expand Down
Loading