Skip to content

Commit

Permalink
Auto merge of #97860 - Dylan-DPC:rollup-t3vxos8, r=Dylan-DPC
Browse files Browse the repository at this point in the history
Rollup of 5 pull requests

Successful merges:

 - #97595 (Remove unwrap from get_vtable)
 - #97597 (Preserve unused pointer to address casts)
 - #97819 (Recover `import` instead of `use` in item)
 - #97823 (Recover missing comma after match arm)
 - #97851 (Use repr(C) when depending on struct layout in ptr tests)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jun 8, 2022
2 parents 64a7aa7 + 1660b4b commit e45d997
Show file tree
Hide file tree
Showing 20 changed files with 335 additions and 65 deletions.
29 changes: 27 additions & 2 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2605,9 +2605,34 @@ pub enum Rvalue<'tcx> {
static_assert_size!(Rvalue<'_>, 40);

impl<'tcx> Rvalue<'tcx> {
/// Returns true if rvalue can be safely removed when the result is unused.
#[inline]
pub fn is_pointer_int_cast(&self) -> bool {
matches!(self, Rvalue::Cast(CastKind::PointerExposeAddress, _, _))
pub fn is_safe_to_remove(&self) -> bool {
match self {
// Pointer to int casts may be side-effects due to exposing the provenance.
// While the model is undecided, we should be conservative. See
// <https://www.ralfj.de/blog/2022/04/11/provenance-exposed.html>
Rvalue::Cast(CastKind::PointerExposeAddress, _, _) => false,

Rvalue::Use(_)
| Rvalue::Repeat(_, _)
| Rvalue::Ref(_, _, _)
| Rvalue::ThreadLocalRef(_)
| Rvalue::AddressOf(_, _)
| Rvalue::Len(_)
| Rvalue::Cast(
CastKind::Misc | CastKind::Pointer(_) | CastKind::PointerFromExposedAddress,
_,
_,
)
| Rvalue::BinaryOp(_, _)
| Rvalue::CheckedBinaryOp(_, _)
| Rvalue::NullaryOp(_, _)
| Rvalue::UnaryOp(_, _)
| Rvalue::Discriminant(_)
| Rvalue::Aggregate(_, _)
| Rvalue::ShallowInitBox(_, _) => true,
}
}
}

Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_mir_dataflow/src/impls/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,13 +244,10 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> {
// Compute the place that we are storing to, if any
let destination = match &statement.kind {
StatementKind::Assign(assign) => {
if assign.1.is_pointer_int_cast() {
// Pointer to int casts may be side-effects due to exposing the provenance.
// While the model is undecided, we should be conservative. See
// <https://www.ralfj.de/blog/2022/04/11/provenance-exposed.html>
None
} else {
if assign.1.is_safe_to_remove() {
Some(assign.0)
} else {
None
}
}
StatementKind::SetDiscriminant { place, .. } | StatementKind::Deinit(place) => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/dead_store_elimination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitS
for (statement_index, statement) in bb_data.statements.iter().enumerate().rev() {
let loc = Location { block: bb, statement_index };
if let StatementKind::Assign(assign) = &statement.kind {
if assign.1.is_pointer_int_cast() {
if !assign.1.is_safe_to_remove() {
continue;
}
}
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_mir_transform/src/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,12 @@ impl<'tcx> Visitor<'tcx> for UsedLocals {
StatementKind::StorageLive(_local) | StatementKind::StorageDead(_local) => {}

StatementKind::Assign(box (ref place, ref rvalue)) => {
self.visit_lhs(place, location);
self.visit_rvalue(rvalue, location);
if rvalue.is_safe_to_remove() {
self.visit_lhs(place, location);
self.visit_rvalue(rvalue, location);
} else {
self.super_statement(statement, location);
}
}

StatementKind::SetDiscriminant { ref place, variant_index: _ }
Expand Down
51 changes: 38 additions & 13 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2718,13 +2718,12 @@ impl<'a> Parser<'a> {
));
}
this.expect_one_of(&[token::Comma], &[token::CloseDelim(Delimiter::Brace)])
.map_err(|mut err| {
match (sm.span_to_lines(expr.span), sm.span_to_lines(arm_start_span)) {
(Ok(ref expr_lines), Ok(ref arm_start_lines))
if arm_start_lines.lines[0].end_col
== expr_lines.lines[0].end_col
&& expr_lines.lines.len() == 2
&& this.token == token::FatArrow =>
.or_else(|mut err| {
if this.token == token::FatArrow {
if let Ok(expr_lines) = sm.span_to_lines(expr.span)
&& let Ok(arm_start_lines) = sm.span_to_lines(arm_start_span)
&& arm_start_lines.lines[0].end_col == expr_lines.lines[0].end_col
&& expr_lines.lines.len() == 2
{
// We check whether there's any trailing code in the parse span,
// if there isn't, we very likely have the following:
Expand All @@ -2743,15 +2742,41 @@ impl<'a> Parser<'a> {
",".to_owned(),
Applicability::MachineApplicable,
);
return Err(err);
}
_ => {
err.span_label(
arrow_span,
"while parsing the `match` arm starting here",
);
} else {
// FIXME(compiler-errors): We could also recover `; PAT =>` here

// Try to parse a following `PAT =>`, if successful
// then we should recover.
let mut snapshot = this.create_snapshot_for_diagnostic();
let pattern_follows = snapshot
.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::EitherTupleOrPipe,
)
.map_err(|err| err.cancel())
.is_ok();
if pattern_follows && snapshot.check(&TokenKind::FatArrow) {
err.cancel();
this.struct_span_err(
hi.shrink_to_hi(),
"expected `,` following `match` arm",
)
.span_suggestion(
hi.shrink_to_hi(),
"missing a comma here to end this `match` arm",
",".to_owned(),
Applicability::MachineApplicable,
)
.emit();
return Ok(true);
}
}
err
err.span_label(arrow_span, "while parsing the `match` arm starting here");
Err(err)
})?;
} else {
this.eat(&token::Comma);
Expand Down
69 changes: 49 additions & 20 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,25 +204,7 @@ impl<'a> Parser<'a> {
let mut def = || mem::replace(def, Defaultness::Final);

let info = if self.eat_keyword(kw::Use) {
// USE ITEM
let tree = self.parse_use_tree()?;

// If wildcard or glob-like brace syntax doesn't have `;`,
// the user may not know `*` or `{}` should be the last.
if let Err(mut e) = self.expect_semi() {
match tree.kind {
UseTreeKind::Glob => {
e.note("the wildcard token must be last on the path");
}
UseTreeKind::Nested(..) => {
e.note("glob-like brace syntax must be last on the path");
}
_ => (),
}
return Err(e);
}

(Ident::empty(), ItemKind::Use(tree))
self.parse_use_item()?
} else if self.check_fn_front_matter(def_final) {
// FUNCTION ITEM
let (ident, sig, generics, body) = self.parse_fn(attrs, fn_parse_mode, lo, vis)?;
Expand Down Expand Up @@ -288,7 +270,12 @@ impl<'a> Parser<'a> {
} else if let IsMacroRulesItem::Yes { has_bang } = self.is_macro_rules_item() {
// MACRO_RULES ITEM
self.parse_item_macro_rules(vis, has_bang)?
} else if vis.kind.is_pub() && self.isnt_macro_invocation() {
} else if self.isnt_macro_invocation()
&& (self.token.is_ident_named(Symbol::intern("import"))
|| self.token.is_ident_named(Symbol::intern("using")))
{
return self.recover_import_as_use();
} else if self.isnt_macro_invocation() && vis.kind.is_pub() {
self.recover_missing_kw_before_item()?;
return Ok(None);
} else if macros_allowed && self.check_path() {
Expand All @@ -300,6 +287,48 @@ impl<'a> Parser<'a> {
Ok(Some(info))
}

fn recover_import_as_use(&mut self) -> PResult<'a, Option<(Ident, ItemKind)>> {
let span = self.token.span;
let token_name = super::token_descr(&self.token);
let snapshot = self.create_snapshot_for_diagnostic();
self.bump();
match self.parse_use_item() {
Ok(u) => {
self.struct_span_err(span, format!("expected item, found {token_name}"))
.span_suggestion_short(
span,
"items are imported using the `use` keyword",
"use".to_owned(),
Applicability::MachineApplicable,
)
.emit();
Ok(Some(u))
}
Err(e) => {
e.cancel();
self.restore_snapshot(snapshot);
Ok(None)
}
}
}

fn parse_use_item(&mut self) -> PResult<'a, (Ident, ItemKind)> {
let tree = self.parse_use_tree()?;
if let Err(mut e) = self.expect_semi() {
match tree.kind {
UseTreeKind::Glob => {
e.note("the wildcard token must be last on the path");
}
UseTreeKind::Nested(..) => {
e.note("glob-like brace syntax must be last on the path");
}
_ => (),
}
return Err(e);
}
Ok((Ident::empty(), ItemKind::Use(tree)))
}

/// When parsing a statement, would the start of a path be an item?
pub(super) fn is_path_start_item(&mut self) -> bool {
self.is_kw_followed_by_ident(kw::Union) // no: `union::b`, yes: `union U { .. }`
Expand Down
14 changes: 8 additions & 6 deletions compiler/rustc_trait_selection/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,22 +304,24 @@ pub fn get_vtable_index_of_object_method<'tcx, N>(
tcx: TyCtxt<'tcx>,
object: &super::ImplSourceObjectData<'tcx, N>,
method_def_id: DefId,
) -> usize {
) -> Option<usize> {
let existential_trait_ref = object
.upcast_trait_ref
.map_bound(|trait_ref| ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref));
let existential_trait_ref = tcx.erase_regions(existential_trait_ref);

// Count number of methods preceding the one we are selecting and
// add them to the total offset.
let index = tcx
if let Some(index) = tcx
.own_existential_vtable_entries(existential_trait_ref)
.iter()
.copied()
.position(|def_id| def_id == method_def_id)
.unwrap_or_else(|| {
bug!("get_vtable_index_of_object_method: {:?} was not found", method_def_id);
});
object.vtable_base + index
{
Some(object.vtable_base + index)
} else {
None
}
}

pub fn closure_trait_ref_and_return_type<'tcx>(
Expand Down
14 changes: 9 additions & 5 deletions compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,11 +349,15 @@ fn resolve_associated_item<'tcx>(
_ => None,
},
traits::ImplSource::Object(ref data) => {
let index = traits::get_vtable_index_of_object_method(tcx, data, trait_item_id);
Some(Instance {
def: ty::InstanceDef::Virtual(trait_item_id, index),
substs: rcvr_substs,
})
if let Some(index) = traits::get_vtable_index_of_object_method(tcx, data, trait_item_id)
{
Some(Instance {
def: ty::InstanceDef::Virtual(trait_item_id, index),
substs: rcvr_substs,
})
} else {
None
}
}
traits::ImplSource::Builtin(..) => {
if Some(trait_ref.def_id) == tcx.lang_items().clone_trait() {
Expand Down
1 change: 1 addition & 0 deletions library/core/tests/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ fn test_const_from_raw_parts() {
#[test]
fn test() {
unsafe {
#[repr(C)]
struct Pair {
fst: isize,
snd: isize,
Expand Down
7 changes: 7 additions & 0 deletions src/test/mir-opt/simplify-locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ fn t4() -> u32 {
unsafe { X + 1 }
}

// EMIT_MIR simplify_locals.expose_addr.SimplifyLocals.diff
fn expose_addr(p: *const usize) {
// Used pointer to address cast. Has a side effect of exposing the provenance.
p as usize;
}

fn main() {
c();
d1();
Expand All @@ -71,4 +77,5 @@ fn main() {
t2();
t3();
t4();
expose_addr(&0);
}
21 changes: 21 additions & 0 deletions src/test/mir-opt/simplify_locals.expose_addr.SimplifyLocals.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
- // MIR for `expose_addr` before SimplifyLocals
+ // MIR for `expose_addr` after SimplifyLocals

fn expose_addr(_1: *const usize) -> () {
debug p => _1; // in scope 0 at $DIR/simplify-locals.rs:66:16: 66:17
let mut _0: (); // return place in scope 0 at $DIR/simplify-locals.rs:66:33: 66:33
let _2: usize; // in scope 0 at $DIR/simplify-locals.rs:68:5: 68:15
let mut _3: *const usize; // in scope 0 at $DIR/simplify-locals.rs:68:5: 68:6

bb0: {
StorageLive(_2); // scope 0 at $DIR/simplify-locals.rs:68:5: 68:15
StorageLive(_3); // scope 0 at $DIR/simplify-locals.rs:68:5: 68:6
_3 = _1; // scope 0 at $DIR/simplify-locals.rs:68:5: 68:6
_2 = move _3 as usize (PointerExposeAddress); // scope 0 at $DIR/simplify-locals.rs:68:5: 68:15
StorageDead(_3); // scope 0 at $DIR/simplify-locals.rs:68:14: 68:15
StorageDead(_2); // scope 0 at $DIR/simplify-locals.rs:68:15: 68:16
_0 = const (); // scope 0 at $DIR/simplify-locals.rs:66:33: 69:2
return; // scope 0 at $DIR/simplify-locals.rs:69:2: 69:2
}
}

15 changes: 15 additions & 0 deletions src/test/ui/did_you_mean/use_instead_of_import.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// run-rustfix

use std::{
//~^ ERROR expected item, found `import`
io::Write,
rc::Rc,
};

pub use std::io;
//~^ ERROR expected item, found `using`

fn main() {
let x = Rc::new(1);
let _ = write!(io::stdout(), "{:?}", x);
}
15 changes: 15 additions & 0 deletions src/test/ui/did_you_mean/use_instead_of_import.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// run-rustfix

import std::{
//~^ ERROR expected item, found `import`
io::Write,
rc::Rc,
};

pub using std::io;
//~^ ERROR expected item, found `using`

fn main() {
let x = Rc::new(1);
let _ = write!(io::stdout(), "{:?}", x);
}
14 changes: 14 additions & 0 deletions src/test/ui/did_you_mean/use_instead_of_import.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: expected item, found `import`
--> $DIR/use_instead_of_import.rs:3:1
|
LL | import std::{
| ^^^^^^ help: items are imported using the `use` keyword

error: expected item, found `using`
--> $DIR/use_instead_of_import.rs:9:5
|
LL | pub using std::io;
| ^^^^^ help: items are imported using the `use` keyword

error: aborting due to 2 previous errors

6 changes: 3 additions & 3 deletions src/test/ui/parser/match-arm-without-braces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ fn main() {
15;
}
match S::get(16) {
Some(Val::Foo) => 17
_ => 18, //~ ERROR expected one of
}
Some(Val::Foo) => 17 //~ ERROR expected `,` following `match` arm
_ => 18,
};
match S::get(19) {
Some(Val::Foo) =>
20; //~ ERROR `match` arm body without braces
Expand Down
Loading

0 comments on commit e45d997

Please sign in to comment.