Skip to content

Commit

Permalink
Unrolled build for rust-lang#123993
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#123993 - compiler-errors:coroutine-obl, r=lcnr

Do `check_coroutine_obligations` once per typeck root

We only need to do `check_coroutine_obligations` once per typeck root, especially since the new solver can't really (easily) associate which obligations correspond to which coroutines.

This requires us to move the checks for sized coroutine fields into `mir_coroutine_witnesses`, but that's fine imo.

r? lcnr
  • Loading branch information
rust-timer committed Apr 23, 2024
2 parents cd90d5c + a8c9a0b commit 8d0d6bf
Show file tree
Hide file tree
Showing 13 changed files with 148 additions and 112 deletions.
47 changes: 11 additions & 36 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@ use rustc_hir as hir;
use rustc_hir::def::{CtorKind, DefKind};
use rustc_hir::Node;
use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
use rustc_infer::traits::{Obligation, TraitEngineExt as _};
use rustc_infer::traits::Obligation;
use rustc_lint_defs::builtin::REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS;
use rustc_middle::middle::resolve_bound_vars::ResolvedArg;
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::traits::ObligationCauseCode;
use rustc_middle::ty::fold::BottomUpFolder;
use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES};
use rustc_middle::ty::util::{Discr, InspectCoroutineFields, IntTypeExt};
Expand All @@ -24,10 +23,10 @@ use rustc_middle::ty::{
};
use rustc_session::lint::builtin::{UNINHABITED_STATIC, UNSUPPORTED_CALLING_CONVENTIONS};
use rustc_target::abi::FieldIdx;
use rustc_trait_selection::traits;
use rustc_trait_selection::traits::error_reporting::on_unimplemented::OnUnimplementedDirective;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _;
use rustc_trait_selection::traits::{self, TraitEngine, TraitEngineExt as _};
use rustc_type_ir::fold::TypeFoldable;

use std::cell::LazyCell;
Expand Down Expand Up @@ -1715,55 +1714,31 @@ fn opaque_type_cycle_error(
err.emit()
}

// FIXME(@lcnr): This should not be computed per coroutine, but instead once for
// each typeck root.
pub(super) fn check_coroutine_obligations(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
) -> Result<(), ErrorGuaranteed> {
debug_assert!(tcx.is_coroutine(def_id.to_def_id()));
debug_assert!(!tcx.is_typeck_child(def_id.to_def_id()));

let typeck = tcx.typeck(def_id);
let param_env = tcx.param_env(typeck.hir_owner.def_id);
let typeck_results = tcx.typeck(def_id);
let param_env = tcx.param_env(def_id);

let coroutine_interior_predicates = &typeck.coroutine_interior_predicates[&def_id];
debug!(?coroutine_interior_predicates);
debug!(?typeck_results.coroutine_stalled_predicates);

let infcx = tcx
.infer_ctxt()
// typeck writeback gives us predicates with their regions erased.
// As borrowck already has checked lifetimes, we do not need to do it again.
.ignoring_regions()
// Bind opaque types to type checking root, as they should have been checked by borrowck,
// but may show up in some cases, like when (root) obligations are stalled in the new solver.
.with_opaque_type_inference(typeck.hir_owner.def_id)
.with_opaque_type_inference(def_id)
.build();

let mut fulfillment_cx = <dyn TraitEngine<'_>>::new(&infcx);
for (predicate, cause) in coroutine_interior_predicates {
let obligation = Obligation::new(tcx, cause.clone(), param_env, *predicate);
fulfillment_cx.register_predicate_obligation(&infcx, obligation);
}

if (tcx.features().unsized_locals || tcx.features().unsized_fn_params)
&& let Some(coroutine) = tcx.mir_coroutine_witnesses(def_id)
{
for field_ty in coroutine.field_tys.iter() {
fulfillment_cx.register_bound(
&infcx,
param_env,
field_ty.ty,
tcx.require_lang_item(hir::LangItem::Sized, Some(field_ty.source_info.span)),
ObligationCause::new(
field_ty.source_info.span,
def_id,
ObligationCauseCode::SizedCoroutineInterior(def_id),
),
);
}
let ocx = ObligationCtxt::new(&infcx);
for (predicate, cause) in &typeck_results.coroutine_stalled_predicates {
ocx.register_obligation(Obligation::new(tcx, cause.clone(), param_env, *predicate));
}

let errors = fulfillment_cx.select_all_or_error(&infcx);
let errors = ocx.select_all_or_error();
debug!(?errors);
if !errors.is_empty() {
return Err(infcx.err_ctxt().report_fulfillment_errors(errors));
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,12 +588,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
obligations
.extend(self.fulfillment_cx.borrow_mut().drain_unstalled_obligations(&self.infcx));

let obligations = obligations.into_iter().map(|o| (o.predicate, o.cause)).collect();
debug!(?obligations);
self.typeck_results
.borrow_mut()
.coroutine_interior_predicates
.insert(expr_def_id, obligations);
let obligations = obligations.into_iter().map(|o| (o.predicate, o.cause));
self.typeck_results.borrow_mut().coroutine_stalled_predicates.extend(obligations);
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use rustc_hir::{
};
use rustc_hir_analysis::collect::suggest_impl_trait;
use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
use rustc_infer::traits::{self};
use rustc_infer::traits;
use rustc_middle::lint::in_external_macro;
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::ty::print::with_no_trimmed_paths;
Expand Down
13 changes: 4 additions & 9 deletions compiler/rustc_hir_typeck/src/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,15 +551,10 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
fn visit_coroutine_interior(&mut self) {
let fcx_typeck_results = self.fcx.typeck_results.borrow();
assert_eq!(fcx_typeck_results.hir_owner, self.typeck_results.hir_owner);
self.tcx().with_stable_hashing_context(move |ref hcx| {
for (&expr_def_id, predicates) in
fcx_typeck_results.coroutine_interior_predicates.to_sorted(hcx, false).into_iter()
{
let predicates =
self.resolve(predicates.clone(), &self.fcx.tcx.def_span(expr_def_id));
self.typeck_results.coroutine_interior_predicates.insert(expr_def_id, predicates);
}
})
for (predicate, cause) in &fcx_typeck_results.coroutine_stalled_predicates {
let (predicate, cause) = self.resolve((*predicate, cause.clone()), &cause.span);
self.typeck_results.coroutine_stalled_predicates.insert((predicate, cause));
}
}

#[instrument(skip(self), level = "debug")]
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,9 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
tcx.hir().par_body_owners(|def_id| {
if tcx.is_coroutine(def_id.to_def_id()) {
tcx.ensure().mir_coroutine_witnesses(def_id);
tcx.ensure().check_coroutine_obligations(def_id);
tcx.ensure().check_coroutine_obligations(
tcx.typeck_root_def_id(def_id.to_def_id()).expect_local(),
);
}
});
sess.time("layout_testing", || layout_test::test_layout(tcx));
Expand Down
11 changes: 4 additions & 7 deletions compiler/rustc_middle/src/ty/typeck_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ use crate::{
GenericArgs, GenericArgsRef, Ty, UserArgs,
},
};
use rustc_data_structures::{
fx::FxIndexMap,
unord::{ExtendUnord, UnordItems, UnordSet},
};
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_data_structures::unord::{ExtendUnord, UnordItems, UnordSet};
use rustc_errors::ErrorGuaranteed;
use rustc_hir::{
self as hir,
Expand Down Expand Up @@ -201,8 +199,7 @@ pub struct TypeckResults<'tcx> {

/// Stores the predicates that apply on coroutine witness types.
/// formatting modified file tests/ui/coroutine/retain-resume-ref.rs
pub coroutine_interior_predicates:
LocalDefIdMap<Vec<(ty::Predicate<'tcx>, ObligationCause<'tcx>)>>,
pub coroutine_stalled_predicates: FxIndexSet<(ty::Predicate<'tcx>, ObligationCause<'tcx>)>,

/// We sometimes treat byte string literals (which are of type `&[u8; N]`)
/// as `&[u8]`, depending on the pattern in which they are used.
Expand Down Expand Up @@ -243,7 +240,7 @@ impl<'tcx> TypeckResults<'tcx> {
closure_min_captures: Default::default(),
closure_fake_reads: Default::default(),
rvalue_scopes: Default::default(),
coroutine_interior_predicates: Default::default(),
coroutine_stalled_predicates: Default::default(),
treat_byte_string_as_slice: Default::default(),
closure_size_eval: Default::default(),
offset_of_data: Default::default(),
Expand Down
40 changes: 40 additions & 0 deletions compiler/rustc_mir_transform/src/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_target::abi::{FieldIdx, VariantIdx};
use rustc_target::spec::PanicStrategy;
use rustc_trait_selection::infer::TyCtxtInferExt as _;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
use rustc_trait_selection::traits::ObligationCtxt;
use rustc_trait_selection::traits::{ObligationCause, ObligationCauseCode};
use std::{iter, ops};

pub struct StateTransform;
Expand Down Expand Up @@ -1584,10 +1588,46 @@ pub(crate) fn mir_coroutine_witnesses<'tcx>(
let (_, coroutine_layout, _) = compute_layout(liveness_info, body);

check_suspend_tys(tcx, &coroutine_layout, body);
check_field_tys_sized(tcx, &coroutine_layout, def_id);

Some(coroutine_layout)
}

fn check_field_tys_sized<'tcx>(
tcx: TyCtxt<'tcx>,
coroutine_layout: &CoroutineLayout<'tcx>,
def_id: LocalDefId,
) {
// No need to check if unsized_locals/unsized_fn_params is disabled,
// since we will error during typeck.
if !tcx.features().unsized_locals && !tcx.features().unsized_fn_params {
return;
}

let infcx = tcx.infer_ctxt().ignoring_regions().build();
let param_env = tcx.param_env(def_id);

let ocx = ObligationCtxt::new(&infcx);
for field_ty in &coroutine_layout.field_tys {
ocx.register_bound(
ObligationCause::new(
field_ty.source_info.span,
def_id,
ObligationCauseCode::SizedCoroutineInterior(def_id),
),
param_env,
field_ty.ty,
tcx.require_lang_item(hir::LangItem::Sized, Some(field_ty.source_info.span)),
);
}

let errors = ocx.select_all_or_error();
debug!(?errors);
if !errors.is_empty() {
infcx.err_ctxt().report_fulfillment_errors(errors);
}
}

impl<'tcx> MirPass<'tcx> for StateTransform {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let Some(old_yield_ty) = body.yield_ty() else {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_span/src/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_data_structures::sync::{IntoDynSyncSend, MappedReadGuard, ReadGuard, R
use rustc_data_structures::unhash::UnhashMap;
use std::fs;
use std::io::{self, BorrowedBuf, Read};
use std::path::{self};
use std::path;

#[cfg(test)]
mod tests;
Expand Down
10 changes: 0 additions & 10 deletions tests/crashes/122552.rs

This file was deleted.

24 changes: 19 additions & 5 deletions tests/ui/coroutine/clone-impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@

struct NonClone;

fn main() {
fn test1() {
let copyable: u32 = 123;
let clonable_0: Vec<u32> = Vec::new();
let clonable_1: Vec<u32> = Vec::new();
let non_clonable: NonClone = NonClone;

let gen_copy_0 = move || {
yield;
drop(copyable);
};
check_copy(&gen_copy_0);
check_clone(&gen_copy_0);
}

fn test2() {
let copyable: u32 = 123;
let gen_copy_1 = move || {
/*
let v = vec!['a'];
Expand All @@ -33,6 +33,10 @@ fn main() {
};
check_copy(&gen_copy_1);
check_clone(&gen_copy_1);
}

fn test3() {
let clonable_0: Vec<u32> = Vec::new();
let gen_clone_0 = move || {
let v = vec!['a'];
yield;
Expand All @@ -43,6 +47,10 @@ fn main() {
//~^ ERROR the trait bound `Vec<u32>: Copy` is not satisfied
//~| ERROR the trait bound `Vec<char>: Copy` is not satisfied
check_clone(&gen_clone_0);
}

fn test4() {
let clonable_1: Vec<u32> = Vec::new();
let gen_clone_1 = move || {
let v = vec!['a'];
/*
Expand All @@ -59,6 +67,10 @@ fn main() {
//~^ ERROR the trait bound `Vec<u32>: Copy` is not satisfied
//~| ERROR the trait bound `Vec<char>: Copy` is not satisfied
check_clone(&gen_clone_1);
}

fn test5() {
let non_clonable: NonClone = NonClone;
let gen_non_clone = move || {
yield;
drop(non_clonable);
Expand All @@ -71,3 +83,5 @@ fn main() {

fn check_copy<T: Copy>(_x: &T) {}
fn check_clone<T: Clone>(_x: &T) {}

fn main() {}
Loading

0 comments on commit 8d0d6bf

Please sign in to comment.