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

Do check_coroutine_obligations once per typeck root #123993

Merged
merged 4 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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,10 +10,9 @@ 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::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 @@ -23,10 +22,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 @@ -1541,55 +1540,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 @@ -575,12 +575,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 @@ -550,15 +550,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(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only thing I'm kinda unsatisfied with is calling check_coroutine_obligations once per coroutine rather than once per typeck root. Not sure if there's a better place to relocate this check, tho.

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
Loading