Skip to content

Commit

Permalink
Auto merge of #94392 - matthiaskrgr:rollup-npscf95, r=matthiaskrgr
Browse files Browse the repository at this point in the history
Rollup of 5 pull requests

Successful merges:

 - #93400 (Do not suggest using a const parameter when there are bounds on an unused type parameter)
 - #93982 (Provide extra note if synthetic type args are specified)
 - #94087 (Remove unused `unsound_ignore_borrow_on_drop`)
 - #94235 (chalk: Fix wrong debrujin index in opaque type handling.)
 - #94306 (Avoid exhausting stack space in dominator compression)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Feb 26, 2022
2 parents 7c3331c + 648a8e3 commit d5a9bc9
Show file tree
Hide file tree
Showing 17 changed files with 266 additions and 115 deletions.
16 changes: 13 additions & 3 deletions compiler/rustc_data_structures/src/graph/dominators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,19 @@ fn compress(
v: PreorderIndex,
) {
assert!(is_processed(v, lastlinked));
let u = ancestor[v];
if is_processed(u, lastlinked) {
compress(ancestor, lastlinked, semi, label, u);
// Compute the processed list of ancestors
//
// We use a heap stack here to avoid recursing too deeply, exhausting the
// stack space.
let mut stack: smallvec::SmallVec<[_; 8]> = smallvec::smallvec![v];
let mut u = ancestor[v];
while is_processed(u, lastlinked) {
stack.push(u);
u = ancestor[u];
}

// Then in reverse order, popping the stack
for &[v, u] in stack.array_windows().rev() {
if semi[label[u]] < semi[label[v]] {
label[v] = label[u];
}
Expand Down
45 changes: 11 additions & 34 deletions compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,38 +10,11 @@ use rustc_middle::mir::*;
/// At present, this is used as a very limited form of alias analysis. For example,
/// `MaybeBorrowedLocals` is used to compute which locals are live during a yield expression for
/// immovable generators.
pub struct MaybeBorrowedLocals {
ignore_borrow_on_drop: bool,
}

impl MaybeBorrowedLocals {
/// A dataflow analysis that records whether a pointer or reference exists that may alias the
/// given local.
pub fn all_borrows() -> Self {
MaybeBorrowedLocals { ignore_borrow_on_drop: false }
}
}
pub struct MaybeBorrowedLocals;

impl MaybeBorrowedLocals {
/// During dataflow analysis, ignore the borrow that may occur when a place is dropped.
///
/// Drop terminators may call custom drop glue (`Drop::drop`), which takes `&mut self` as a
/// parameter. In the general case, a drop impl could launder that reference into the
/// surrounding environment through a raw pointer, thus creating a valid `*mut` pointing to the
/// dropped local. We are not yet willing to declare this particular case UB, so we must treat
/// all dropped locals as mutably borrowed for now. See discussion on [#61069].
///
/// In some contexts, we know that this borrow will never occur. For example, during
/// const-eval, custom drop glue cannot be run. Code that calls this should document the
/// assumptions that justify ignoring `Drop` terminators in this way.
///
/// [#61069]: https://github.com/rust-lang/rust/pull/61069
pub fn unsound_ignore_borrow_on_drop(self) -> Self {
MaybeBorrowedLocals { ignore_borrow_on_drop: true, ..self }
}

fn transfer_function<'a, T>(&'a self, trans: &'a mut T) -> TransferFunction<'a, T> {
TransferFunction { trans, ignore_borrow_on_drop: self.ignore_borrow_on_drop }
TransferFunction { trans }
}
}

Expand Down Expand Up @@ -92,7 +65,6 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeBorrowedLocals {
/// A `Visitor` that defines the transfer function for `MaybeBorrowedLocals`.
struct TransferFunction<'a, T> {
trans: &'a mut T,
ignore_borrow_on_drop: bool,
}

impl<'tcx, T> Visitor<'tcx> for TransferFunction<'_, T>
Expand Down Expand Up @@ -146,10 +118,15 @@ where
match terminator.kind {
mir::TerminatorKind::Drop { place: dropped_place, .. }
| mir::TerminatorKind::DropAndReplace { place: dropped_place, .. } => {
// See documentation for `unsound_ignore_borrow_on_drop` for an explanation.
if !self.ignore_borrow_on_drop {
self.trans.gen(dropped_place.local);
}
// Drop terminators may call custom drop glue (`Drop::drop`), which takes `&mut
// self` as a parameter. In the general case, a drop impl could launder that
// reference into the surrounding environment through a raw pointer, thus creating
// a valid `*mut` pointing to the dropped local. We are not yet willing to declare
// this particular case UB, so we must treat all dropped locals as mutably borrowed
// for now. See discussion on [#61069].
//
// [#61069]: https://github.com/rust-lang/rust/pull/61069
self.trans.gen(dropped_place.local);
}

TerminatorKind::Abort
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_mir_transform/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,10 +463,8 @@ fn locals_live_across_suspend_points<'tcx>(

// Calculate the MIR locals which have been previously
// borrowed (even if they are still active).
let borrowed_locals_results = MaybeBorrowedLocals::all_borrows()
.into_engine(tcx, body_ref)
.pass_name("generator")
.iterate_to_fixpoint();
let borrowed_locals_results =
MaybeBorrowedLocals.into_engine(tcx, body_ref).pass_name("generator").iterate_to_fixpoint();

let mut borrowed_locals_cursor =
rustc_mir_dataflow::ResultsCursor::new(body_ref, &borrowed_locals_results);
Expand Down
55 changes: 40 additions & 15 deletions compiler/rustc_traits/src/chalk/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

use rustc_middle::traits::ChalkRustInterner as RustInterner;
use rustc_middle::ty::subst::{InternalSubsts, Subst, SubstsRef};
use rustc_middle::ty::{self, AssocItemContainer, AssocKind, TyCtxt, TypeFoldable};
use rustc_middle::ty::{self, AssocItemContainer, AssocKind, Ty, TyCtxt, TypeFoldable};

use rustc_ast::ast;
use rustc_attr as attr;
Expand Down Expand Up @@ -482,21 +482,11 @@ impl<'tcx> chalk_solve::RustIrDatabase<RustInterner<'tcx>> for RustIrDatabase<'t
.iter()
.map(|(bound, _)| bound.subst(self.interner.tcx, &bound_vars))
.map(|bound| {
bound.fold_with(&mut ty::fold::BottomUpFolder {
bound.fold_with(&mut ReplaceOpaqueTyFolder {
tcx: self.interner.tcx,
ty_op: |ty| {
if let ty::Opaque(def_id, substs) = *ty.kind() {
if def_id == opaque_ty_id.0 && substs == identity_substs {
return self.interner.tcx.mk_ty(ty::Bound(
ty::INNERMOST,
ty::BoundTy::from(ty::BoundVar::from_u32(0)),
));
}
}
ty
},
lt_op: |lt| lt,
ct_op: |ct| ct,
opaque_ty_id,
identity_substs,
binder_index: ty::INNERMOST,
})
})
.filter_map(|bound| {
Expand Down Expand Up @@ -739,3 +729,38 @@ fn binders_for<'tcx>(
}),
)
}

struct ReplaceOpaqueTyFolder<'tcx> {
tcx: TyCtxt<'tcx>,
opaque_ty_id: chalk_ir::OpaqueTyId<RustInterner<'tcx>>,
identity_substs: SubstsRef<'tcx>,
binder_index: ty::DebruijnIndex,
}

impl<'tcx> ty::TypeFolder<'tcx> for ReplaceOpaqueTyFolder<'tcx> {
fn tcx<'b>(&'b self) -> TyCtxt<'tcx> {
self.tcx
}

fn fold_binder<T: TypeFoldable<'tcx>>(
&mut self,
t: ty::Binder<'tcx, T>,
) -> ty::Binder<'tcx, T> {
self.binder_index.shift_in(1);
let t = t.super_fold_with(self);
self.binder_index.shift_out(1);
t
}

fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
if let ty::Opaque(def_id, substs) = *ty.kind() {
if def_id == self.opaque_ty_id.0 && substs == self.identity_substs {
return self.tcx.mk_ty(ty::Bound(
self.binder_index,
ty::BoundTy::from(ty::BoundVar::from_u32(0)),
));
}
}
ty
}
}
5 changes: 4 additions & 1 deletion compiler/rustc_traits/src/chalk/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,10 @@ impl<'tcx> LowerInto<'tcx, chalk_ir::Ty<RustInterner<'tcx>>> for Ty<'tcx> {
ty::Closure(def_id, substs) => {
chalk_ir::TyKind::Closure(chalk_ir::ClosureId(def_id), substs.lower_into(interner))
}
ty::Generator(_def_id, _substs, _) => unimplemented!(),
ty::Generator(def_id, substs, _) => chalk_ir::TyKind::Generator(
chalk_ir::GeneratorId(def_id),
substs.lower_into(interner),
),
ty::GeneratorWitness(_) => unimplemented!(),
ty::Never => chalk_ir::TyKind::Never,
ty::Tuple(types) => {
Expand Down
107 changes: 58 additions & 49 deletions compiler/rustc_typeck/src/astconv/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,61 +512,69 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
explicit_late_bound == ExplicitLateBound::Yes,
);

let mut check_types_and_consts =
|expected_min, expected_max, provided, params_offset, args_offset| {
debug!(
?expected_min,
?expected_max,
?provided,
?params_offset,
?args_offset,
"check_types_and_consts"
let mut check_types_and_consts = |expected_min,
expected_max,
expected_max_with_synth,
provided,
params_offset,
args_offset| {
debug!(
?expected_min,
?expected_max,
?provided,
?params_offset,
?args_offset,
"check_types_and_consts"
);
if (expected_min..=expected_max).contains(&provided) {
return true;
}

let num_default_params = expected_max - expected_min;

let gen_args_info = if provided > expected_max {
invalid_args.extend(
gen_args.args[args_offset + expected_max..args_offset + provided]
.iter()
.map(|arg| arg.span()),
);
if (expected_min..=expected_max).contains(&provided) {
return true;
let num_redundant_args = provided - expected_max;

// Provide extra note if synthetic arguments like `impl Trait` are specified.
let synth_provided = provided <= expected_max_with_synth;

GenericArgsInfo::ExcessTypesOrConsts {
num_redundant_args,
num_default_params,
args_offset,
synth_provided,
}
} else {
let num_missing_args = expected_max - provided;

let num_default_params = expected_max - expected_min;
GenericArgsInfo::MissingTypesOrConsts {
num_missing_args,
num_default_params,
args_offset,
}
};

let gen_args_info = if provided > expected_max {
invalid_args.extend(
gen_args.args[args_offset + expected_max..args_offset + provided]
.iter()
.map(|arg| arg.span()),
);
let num_redundant_args = provided - expected_max;
debug!(?gen_args_info);

GenericArgsInfo::ExcessTypesOrConsts {
num_redundant_args,
num_default_params,
args_offset,
}
} else {
let num_missing_args = expected_max - provided;
WrongNumberOfGenericArgs::new(
tcx,
gen_args_info,
seg,
gen_params,
params_offset,
gen_args,
def_id,
)
.diagnostic()
.emit_unless(gen_args.has_err());

GenericArgsInfo::MissingTypesOrConsts {
num_missing_args,
num_default_params,
args_offset,
}
};

debug!(?gen_args_info);

WrongNumberOfGenericArgs::new(
tcx,
gen_args_info,
seg,
gen_params,
params_offset,
gen_args,
def_id,
)
.diagnostic()
.emit_unless(gen_args.has_err());

false
};
false
};

let args_correct = {
let expected_min = if infer_args {
Expand All @@ -582,6 +590,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
check_types_and_consts(
expected_min,
param_counts.consts + named_type_param_count,
param_counts.consts + named_type_param_count + synth_type_param_count,
gen_args.num_generic_params(),
param_counts.lifetimes + has_self as usize,
gen_args.num_lifetime_params(),
Expand Down
36 changes: 32 additions & 4 deletions compiler/rustc_typeck/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode,

use std::convert::TryInto;
use std::iter;
use std::lazy::Lazy;
use std::ops::ControlFlow;

/// Helper type of a temporary returned by `.for_item(...)`.
Expand Down Expand Up @@ -1720,8 +1721,29 @@ fn check_variances_for_type_defn<'tcx>(

identify_constrained_generic_params(tcx, ty_predicates, None, &mut constrained_parameters);

// Lazily calculated because it is only needed in case of an error.
let explicitly_bounded_params = Lazy::new(|| {
let icx = crate::collect::ItemCtxt::new(tcx, item.def_id.to_def_id());
hir_generics
.where_clause
.predicates
.iter()
.filter_map(|predicate| match predicate {
hir::WherePredicate::BoundPredicate(predicate) => {
match icx.to_ty(predicate.bounded_ty).kind() {
ty::Param(data) => Some(Parameter(data.index)),
_ => None,
}
}
_ => None,
})
.collect::<FxHashSet<_>>()
});

for (index, _) in variances.iter().enumerate() {
if constrained_parameters.contains(&Parameter(index as u32)) {
let parameter = Parameter(index as u32);

if constrained_parameters.contains(&parameter) {
continue;
}

Expand All @@ -1730,13 +1752,19 @@ fn check_variances_for_type_defn<'tcx>(
match param.name {
hir::ParamName::Error => {}
_ => {
report_bivariance(tcx, param);
let has_explicit_bounds =
!param.bounds.is_empty() || explicitly_bounded_params.contains(&parameter);
report_bivariance(tcx, param, has_explicit_bounds);
}
}
}
}

fn report_bivariance(tcx: TyCtxt<'_>, param: &rustc_hir::GenericParam<'_>) -> ErrorReported {
fn report_bivariance(
tcx: TyCtxt<'_>,
param: &rustc_hir::GenericParam<'_>,
has_explicit_bounds: bool,
) -> ErrorReported {
let span = param.span;
let param_name = param.name.ident().name;
let mut err = error_392(tcx, span, param_name);
Expand All @@ -1754,7 +1782,7 @@ fn report_bivariance(tcx: TyCtxt<'_>, param: &rustc_hir::GenericParam<'_>) -> Er
};
err.help(&msg);

if matches!(param.kind, rustc_hir::GenericParamKind::Type { .. }) {
if matches!(param.kind, hir::GenericParamKind::Type { .. }) && !has_explicit_bounds {
err.help(&format!(
"if you intended `{0}` to be a const parameter, use `const {0}: usize` instead",
param_name
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ This API is completely unstable and subject to change.
#![feature(slice_partition_dedup)]
#![feature(control_flow_enum)]
#![feature(hash_drain_filter)]
#![feature(once_cell)]
#![recursion_limit = "256"]
#![allow(rustc::potential_query_instability)]

Expand Down
Loading

0 comments on commit d5a9bc9

Please sign in to comment.