Skip to content

Commit

Permalink
Auto merge of rust-lang#107064 - GuillaumeGomez:rollup-pbgu6r3, r=Gui…
Browse files Browse the repository at this point in the history
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#105977 (Transform async `ResumeTy` in generator transform)
 - rust-lang#106927 (make `CastError::NeedsDeref` create a `MachineApplicable` suggestion)
 - rust-lang#106931 (document + UI test `E0208` and make its output more user-friendly)
 - rust-lang#107027 (Remove extra removal from test path)
 - rust-lang#107037 (Fix Dominators::rank_partial_cmp to match documentation)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jan 19, 2023
2 parents 705a96d + dee88e0 commit 79335f1
Show file tree
Hide file tree
Showing 33 changed files with 716 additions and 100 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_data_structures/src/graph/dominators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ impl<Node: Idx> Dominators<Node> {
/// of two unrelated nodes will also be consistent, but otherwise the order has no
/// meaning.) This method cannot be used to determine if either Node dominates the other.
pub fn rank_partial_cmp(&self, lhs: Node, rhs: Node) -> Option<Ordering> {
self.post_order_rank[lhs].partial_cmp(&self.post_order_rank[rhs])
self.post_order_rank[rhs].partial_cmp(&self.post_order_rank[lhs])
}
}

Expand Down
45 changes: 45 additions & 0 deletions compiler/rustc_error_codes/src/error_codes/E0208.md
Original file line number Diff line number Diff line change
@@ -1 +1,46 @@
#### This error code is internal to the compiler and will not be emitted with normal Rust code.
#### Note: this error code is no longer emitted by the compiler.

This error code shows the variance of a type's generic parameters.

Erroneous code example:

```compile_fail
// NOTE: this feature is perma-unstable and should *only* be used for
// testing purposes.
#![feature(rustc_attrs)]
#[rustc_variance]
struct Foo<'a, T> { // error: deliberate error to display type's variance
t: &'a mut T,
}
```

which produces the following error:

```text
error: [-, o]
--> <anon>:4:1
|
4 | struct Foo<'a, T> {
| ^^^^^^^^^^^^^^^^^
```

*Note that while `#[rustc_variance]` still exists and is used within the*
*compiler, it no longer is marked as `E0208` and instead has no error code.*

This error is deliberately triggered with the `#[rustc_variance]` attribute
(`#![feature(rustc_attrs)]` must be enabled) and helps to show you the variance
of the type's generic parameters. You can read more about variance and
subtyping in [this section of the Rustnomicon]. For a more in depth look at
variance (including a more complete list of common variances) see
[this section of the Reference]. For information on how variance is implemented
in the compiler, see [this section of `rustc-dev-guide`].

This error can be easily fixed by removing the `#[rustc_variance]` attribute,
the compiler's suggestion to comment it out can be applied automatically with
`rustfix`.

[this section of the Rustnomicon]: https://doc.rust-lang.org/nomicon/subtyping.html
[this section of the Reference]: https://doc.rust-lang.org/reference/subtyping.html#variance
[this section of `rustc-dev-guide`]: https://rustc-dev-guide.rust-lang.org/variance.html
8 changes: 8 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1787,6 +1787,14 @@ impl Expr<'_> {
expr
}

pub fn peel_borrows(&self) -> &Self {
let mut expr = self;
while let ExprKind::AddrOf(.., inner) = &expr.kind {
expr = inner;
}
expr
}

pub fn can_have_side_effects(&self) -> bool {
match self.peel_drop_temps().kind {
ExprKind::Path(_) | ExprKind::Lit(_) => false,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ language_item_table! {
IdentityFuture, sym::identity_future, identity_future_fn, Target::Fn, GenericRequirement::None;
GetContext, sym::get_context, get_context_fn, Target::Fn, GenericRequirement::None;

Context, sym::Context, context, Target::Struct, GenericRequirement::None;
FuturePoll, sym::poll, future_poll_fn, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None;

FromFrom, sym::from, from_fn, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None;
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_hir_analysis/src/variance/test.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use rustc_errors::struct_span_err;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::sym;

Expand All @@ -8,8 +7,8 @@ pub fn test_variance(tcx: TyCtxt<'_>) {
for id in tcx.hir().items() {
if tcx.has_attr(id.owner_id.to_def_id(), sym::rustc_variance) {
let variances_of = tcx.variances_of(id.owner_id);
struct_span_err!(tcx.sess, tcx.def_span(id.owner_id), E0208, "{:?}", variances_of)
.emit();

tcx.sess.struct_span_err(tcx.def_span(id.owner_id), format!("{variances_of:?}")).emit();
}
}
}
38 changes: 21 additions & 17 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use super::FnCtxt;

use crate::type_error_struct;
use hir::ExprKind;
use rustc_errors::{
struct_span_err, Applicability, DelayDm, Diagnostic, DiagnosticBuilder, ErrorGuaranteed,
};
Expand Down Expand Up @@ -151,7 +152,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

#[derive(Copy, Clone)]
pub enum CastError {
ErrorGuaranteed,
ErrorGuaranteed(ErrorGuaranteed),

CastToBool,
CastToChar,
Expand All @@ -176,8 +177,8 @@ pub enum CastError {
}

impl From<ErrorGuaranteed> for CastError {
fn from(_: ErrorGuaranteed) -> Self {
CastError::ErrorGuaranteed
fn from(err: ErrorGuaranteed) -> Self {
CastError::ErrorGuaranteed(err)
}
}

Expand Down Expand Up @@ -225,33 +226,36 @@ impl<'a, 'tcx> CastCheck<'tcx> {

fn report_cast_error(&self, fcx: &FnCtxt<'a, 'tcx>, e: CastError) {
match e {
CastError::ErrorGuaranteed => {
CastError::ErrorGuaranteed(_) => {
// an error has already been reported
}
CastError::NeedDeref => {
let error_span = self.span;
let mut err = make_invalid_casting_error(
fcx.tcx.sess,
self.span,
self.expr_ty,
self.cast_ty,
fcx,
);
let cast_ty = fcx.ty_to_string(self.cast_ty);
err.span_label(
error_span,
format!("cannot cast `{}` as `{}`", fcx.ty_to_string(self.expr_ty), cast_ty),
);
if let Ok(snippet) = fcx.sess().source_map().span_to_snippet(self.expr_span) {
err.span_suggestion(
self.expr_span,
"dereference the expression",
format!("*{}", snippet),
Applicability::MaybeIncorrect,

if matches!(self.expr.kind, ExprKind::AddrOf(..)) {
// get just the borrow part of the expression
let span = self.expr_span.with_hi(self.expr.peel_borrows().span.lo());
err.span_suggestion_verbose(
span,
"remove the unneeded borrow",
"",
Applicability::MachineApplicable,
);
} else {
err.span_help(self.expr_span, "dereference the expression with `*`");
err.span_suggestion_verbose(
self.expr_span.shrink_to_lo(),
"dereference the expression",
"*",
Applicability::MachineApplicable,
);
}

err.emit();
}
CastError::NeedViaThinPtr | CastError::NeedViaPtr => {
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1952,6 +1952,15 @@ impl<'tcx> TyCtxt<'tcx> {
self.mk_ty(GeneratorWitness(types))
}

/// Creates a `&mut Context<'_>` [`Ty`] with erased lifetimes.
pub fn mk_task_context(self) -> Ty<'tcx> {
let context_did = self.require_lang_item(LangItem::Context, None);
let context_adt_ref = self.adt_def(context_did);
let context_substs = self.intern_substs(&[self.lifetimes.re_erased.into()]);
let context_ty = self.mk_adt(context_adt_ref, context_substs);
self.mk_mut_ref(self.lifetimes.re_erased, context_ty)
}

#[inline]
pub fn mk_ty_var(self, v: TyVid) -> Ty<'tcx> {
self.mk_ty_infer(TyVar(v))
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,11 +341,11 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
if a.is_in_same_bcb(b) {
Some(Ordering::Equal)
} else {
// Sort equal spans by dominator relationship, in reverse order (so
// dominators always come after the dominated equal spans). When later
// comparing two spans in order, the first will either dominate the second,
// or they will have no dominator relationship.
self.basic_coverage_blocks.dominators().rank_partial_cmp(b.bcb, a.bcb)
// Sort equal spans by dominator relationship (so dominators always come
// before the dominated equal spans). When later comparing two spans in
// order, the first will either dominate the second, or they will have no
// dominator relationship.
self.basic_coverage_blocks.dominators().rank_partial_cmp(a.bcb, b.bcb)
}
} else {
// Sort hi() in reverse order so shorter spans are attempted after longer spans.
Expand Down
118 changes: 111 additions & 7 deletions compiler/rustc_mir_transform/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,104 @@ fn replace_local<'tcx>(
new_local
}

/// Transforms the `body` of the generator applying the following transforms:
///
/// - Eliminates all the `get_context` calls that async lowering created.
/// - Replace all `Local` `ResumeTy` types with `&mut Context<'_>` (`context_mut_ref`).
///
/// The `Local`s that have their types replaced are:
/// - The `resume` argument itself.
/// - The argument to `get_context`.
/// - The yielded value of a `yield`.
///
/// The `ResumeTy` hides a `&mut Context<'_>` behind an unsafe raw pointer, and the
/// `get_context` function is being used to convert that back to a `&mut Context<'_>`.
///
/// Ideally the async lowering would not use the `ResumeTy`/`get_context` indirection,
/// but rather directly use `&mut Context<'_>`, however that would currently
/// lead to higher-kinded lifetime errors.
/// See <https://github.com/rust-lang/rust/issues/105501>.
///
/// The async lowering step and the type / lifetime inference / checking are
/// still using the `ResumeTy` indirection for the time being, and that indirection
/// is removed here. After this transform, the generator body only knows about `&mut Context<'_>`.
fn transform_async_context<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let context_mut_ref = tcx.mk_task_context();

// replace the type of the `resume` argument
replace_resume_ty_local(tcx, body, Local::new(2), context_mut_ref);

let get_context_def_id = tcx.require_lang_item(LangItem::GetContext, None);

for bb in BasicBlock::new(0)..body.basic_blocks.next_index() {
let bb_data = &body[bb];
if bb_data.is_cleanup {
continue;
}

match &bb_data.terminator().kind {
TerminatorKind::Call { func, .. } => {
let func_ty = func.ty(body, tcx);
if let ty::FnDef(def_id, _) = *func_ty.kind() {
if def_id == get_context_def_id {
let local = eliminate_get_context_call(&mut body[bb]);
replace_resume_ty_local(tcx, body, local, context_mut_ref);
}
} else {
continue;
}
}
TerminatorKind::Yield { resume_arg, .. } => {
replace_resume_ty_local(tcx, body, resume_arg.local, context_mut_ref);
}
_ => {}
}
}
}

fn eliminate_get_context_call<'tcx>(bb_data: &mut BasicBlockData<'tcx>) -> Local {
let terminator = bb_data.terminator.take().unwrap();
if let TerminatorKind::Call { mut args, destination, target, .. } = terminator.kind {
let arg = args.pop().unwrap();
let local = arg.place().unwrap().local;

let arg = Rvalue::Use(arg);
let assign = Statement {
source_info: terminator.source_info,
kind: StatementKind::Assign(Box::new((destination, arg))),
};
bb_data.statements.push(assign);
bb_data.terminator = Some(Terminator {
source_info: terminator.source_info,
kind: TerminatorKind::Goto { target: target.unwrap() },
});
local
} else {
bug!();
}
}

#[cfg_attr(not(debug_assertions), allow(unused))]
fn replace_resume_ty_local<'tcx>(
tcx: TyCtxt<'tcx>,
body: &mut Body<'tcx>,
local: Local,
context_mut_ref: Ty<'tcx>,
) {
let local_ty = std::mem::replace(&mut body.local_decls[local].ty, context_mut_ref);
// We have to replace the `ResumeTy` that is used for type and borrow checking
// with `&mut Context<'_>` in MIR.
#[cfg(debug_assertions)]
{
if let ty::Adt(resume_ty_adt, _) = local_ty.kind() {
let expected_adt = tcx.adt_def(tcx.require_lang_item(LangItem::ResumeTy, None));
assert_eq!(*resume_ty_adt, expected_adt);
} else {
panic!("expected `ResumeTy`, found `{:?}`", local_ty);
};
}
}

struct LivenessInfo {
/// Which locals are live across any suspension point.
saved_locals: GeneratorSavedLocals,
Expand Down Expand Up @@ -1283,13 +1381,13 @@ impl<'tcx> MirPass<'tcx> for StateTransform {
}
};

let is_async_kind = body.generator_kind().unwrap() != GeneratorKind::Gen;
let is_async_kind = matches!(body.generator_kind(), Some(GeneratorKind::Async(_)));
let (state_adt_ref, state_substs) = if is_async_kind {
// Compute Poll<return_ty>
let state_did = tcx.require_lang_item(LangItem::Poll, None);
let state_adt_ref = tcx.adt_def(state_did);
let state_substs = tcx.intern_substs(&[body.return_ty().into()]);
(state_adt_ref, state_substs)
let poll_did = tcx.require_lang_item(LangItem::Poll, None);
let poll_adt_ref = tcx.adt_def(poll_did);
let poll_substs = tcx.intern_substs(&[body.return_ty().into()]);
(poll_adt_ref, poll_substs)
} else {
// Compute GeneratorState<yield_ty, return_ty>
let state_did = tcx.require_lang_item(LangItem::GeneratorState, None);
Expand All @@ -1303,13 +1401,19 @@ impl<'tcx> MirPass<'tcx> for StateTransform {
// RETURN_PLACE then is a fresh unused local with type ret_ty.
let new_ret_local = replace_local(RETURN_PLACE, ret_ty, body, tcx);

// Replace all occurrences of `ResumeTy` with `&mut Context<'_>` within async bodies.
if is_async_kind {
transform_async_context(tcx, body);
}

// We also replace the resume argument and insert an `Assign`.
// This is needed because the resume argument `_2` might be live across a `yield`, in which
// case there is no `Assign` to it that the transform can turn into a store to the generator
// state. After the yield the slot in the generator state would then be uninitialized.
let resume_local = Local::new(2);
let new_resume_local =
replace_local(resume_local, body.local_decls[resume_local].ty, body, tcx);
let resume_ty =
if is_async_kind { tcx.mk_task_context() } else { body.local_decls[resume_local].ty };
let new_resume_local = replace_local(resume_local, resume_ty, body, tcx);

// When first entering the generator, move the resume argument into its new local.
let source_info = SourceInfo::outermost(body.span);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ symbols! {
Capture,
Center,
Clone,
Context,
Continue,
Copy,
Count,
Expand Down
Loading

0 comments on commit 79335f1

Please sign in to comment.