Skip to content

Commit

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

Successful merges:

 - #121236 (Don't show suggestion if slice pattern is not top-level)
 - #121787 (run change tracker even when config parse fails)
 - #122633 (avoid unnecessary collect())

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Mar 17, 2024
2 parents a0c20d5 + 1213746 commit 35dfc67
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 31 deletions.
5 changes: 1 addition & 4 deletions compiler/rustc_hir_analysis/src/astconv/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,10 +408,7 @@ impl<'tcx> dyn AstConv<'tcx> + '_ {
traits with associated type `{name}`, you could use the \
fully-qualified path",
),
traits
.iter()
.map(|trait_str| format!("<Example as {trait_str}>::{name}"))
.collect::<Vec<_>>(),
traits.iter().map(|trait_str| format!("<Example as {trait_str}>::{name}")),
Applicability::HasPlaceholders,
);
}
Expand Down
40 changes: 27 additions & 13 deletions compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ struct PatInfo<'tcx, 'a> {
binding_mode: BindingMode,
top_info: TopInfo<'tcx>,
decl_origin: Option<DeclOrigin<'a>>,

/// The depth of current pattern
current_depth: u32,
}

impl<'tcx> FnCtxt<'_, 'tcx> {
Expand Down Expand Up @@ -152,7 +155,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
decl_origin: Option<DeclOrigin<'tcx>>,
) {
let info = TopInfo { expected, origin_expr, span };
let pat_info = PatInfo { binding_mode: INITIAL_BM, top_info: info, decl_origin };
let pat_info =
PatInfo { binding_mode: INITIAL_BM, top_info: info, decl_origin, current_depth: 0 };
self.check_pat(pat, expected, pat_info);
}

Expand All @@ -163,7 +167,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Conversely, inside this module, `check_pat_top` should never be used.
#[instrument(level = "debug", skip(self, pat_info))]
fn check_pat(&self, pat: &'tcx Pat<'tcx>, expected: Ty<'tcx>, pat_info: PatInfo<'tcx, '_>) {
let PatInfo { binding_mode: def_bm, top_info: ti, .. } = pat_info;
let PatInfo { binding_mode: def_bm, top_info: ti, current_depth, .. } = pat_info;

let path_res = match &pat.kind {
PatKind::Path(qpath) => Some(
self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span, None),
Expand All @@ -172,8 +177,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};
let adjust_mode = self.calc_adjust_mode(pat, path_res.map(|(res, ..)| res));
let (expected, def_bm) = self.calc_default_binding_mode(pat, expected, def_bm, adjust_mode);
let pat_info =
PatInfo { binding_mode: def_bm, top_info: ti, decl_origin: pat_info.decl_origin };
let pat_info = PatInfo {
binding_mode: def_bm,
top_info: ti,
decl_origin: pat_info.decl_origin,
current_depth: current_depth + 1,
};

let ty = match pat.kind {
PatKind::Wild | PatKind::Err(_) => expected,
Expand Down Expand Up @@ -1046,14 +1055,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected: Ty<'tcx>,
pat_info: PatInfo<'tcx, '_>,
) -> Ty<'tcx> {
let PatInfo { binding_mode: def_bm, top_info: ti, decl_origin } = pat_info;
let PatInfo { binding_mode: def_bm, top_info: ti, decl_origin, current_depth } = pat_info;
let tcx = self.tcx;
let on_error = |e| {
for pat in subpats {
self.check_pat(
pat,
Ty::new_error(tcx, e),
PatInfo { binding_mode: def_bm, top_info: ti, decl_origin },
PatInfo { binding_mode: def_bm, top_info: ti, decl_origin, current_depth },
);
}
};
Expand Down Expand Up @@ -1120,7 +1129,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.check_pat(
subpat,
field_ty,
PatInfo { binding_mode: def_bm, top_info: ti, decl_origin },
PatInfo { binding_mode: def_bm, top_info: ti, decl_origin, current_depth },
);

self.tcx.check_stability(
Expand Down Expand Up @@ -2134,7 +2143,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// The expected type must be an array or slice, but was neither, so error.
_ => {
let guar = expected.error_reported().err().unwrap_or_else(|| {
self.error_expected_array_or_slice(span, expected, pat_info.top_info)
self.error_expected_array_or_slice(span, expected, pat_info)
});
let err = Ty::new_error(self.tcx, guar);
(err, Some(err), err)
Expand Down Expand Up @@ -2273,8 +2282,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
span: Span,
expected_ty: Ty<'tcx>,
ti: TopInfo<'tcx>,
pat_info: PatInfo<'tcx, '_>,
) -> ErrorGuaranteed {
let PatInfo { top_info: ti, current_depth, .. } = pat_info;

let mut err = struct_span_code_err!(
self.dcx(),
span,
Expand All @@ -2292,9 +2303,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&& let Some(_) = ti.origin_expr
&& let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span)
{
let ty = self.resolve_vars_if_possible(ti.expected);
let is_slice_or_array_or_vector = self.is_slice_or_array_or_vector(ty);
match is_slice_or_array_or_vector.1.kind() {
let resolved_ty = self.resolve_vars_if_possible(ti.expected);
let (is_slice_or_array_or_vector, resolved_ty) =
self.is_slice_or_array_or_vector(resolved_ty);
match resolved_ty.kind() {
ty::Adt(adt_def, _)
if self.tcx.is_diagnostic_item(sym::Option, adt_def.did())
|| self.tcx.is_diagnostic_item(sym::Result, adt_def.did()) =>
Expand All @@ -2309,7 +2321,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
_ => (),
}
if is_slice_or_array_or_vector.0 {

let is_top_level = current_depth <= 1;
if is_slice_or_array_or_vector && is_top_level {
err.span_suggestion(
span,
"consider slicing here",
Expand Down
12 changes: 3 additions & 9 deletions src/bootstrap/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ use std::{
};

use bootstrap::{
find_recent_config_change_ids, t, Build, Config, Subcommand, CONFIG_CHANGE_HISTORY,
find_recent_config_change_ids, human_readable_changes, t, Build, Config, Subcommand,
CONFIG_CHANGE_HISTORY,
};

fn main() {
Expand Down Expand Up @@ -164,14 +165,7 @@ fn check_version(config: &Config) -> Option<String> {
}

msg.push_str("There have been changes to x.py since you last updated:\n");

for change in changes {
msg.push_str(&format!(" [{}] {}\n", change.severity, change.summary));
msg.push_str(&format!(
" - PR Link https://github.com/rust-lang/rust/pull/{}\n",
change.change_id
));
}
msg.push_str(&human_readable_changes(&changes));

msg.push_str("NOTE: to silence this warning, ");
msg.push_str(&format!(
Expand Down
31 changes: 28 additions & 3 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,8 @@ impl Target {
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
pub(crate) struct TomlConfig {
changelog_seen: Option<usize>, // FIXME: Deprecated field. Remove it at 2024.
change_id: Option<usize>,
#[serde(flatten)]
change_id: ChangeIdWrapper,
build: Option<Build>,
install: Option<Install>,
llvm: Option<Llvm>,
Expand All @@ -616,6 +617,16 @@ pub(crate) struct TomlConfig {
profile: Option<String>,
}

/// Since we use `#[serde(deny_unknown_fields)]` on `TomlConfig`, we need a wrapper type
/// for the "change-id" field to parse it even if other fields are invalid. This ensures
/// that if deserialization fails due to other fields, we can still provide the changelogs
/// to allow developers to potentially find the reason for the failure in the logs..
#[derive(Deserialize, Default)]
pub(crate) struct ChangeIdWrapper {
#[serde(alias = "change-id")]
pub(crate) inner: Option<usize>,
}

/// Describes how to handle conflicts in merging two [`TomlConfig`]
#[derive(Copy, Clone, Debug)]
enum ReplaceOpt {
Expand Down Expand Up @@ -657,7 +668,7 @@ impl Merge for TomlConfig {
}
}
self.changelog_seen.merge(changelog_seen, replace);
self.change_id.merge(change_id, replace);
self.change_id.inner.merge(change_id.inner, replace);
do_merge(&mut self.build, build, replace);
do_merge(&mut self.install, install, replace);
do_merge(&mut self.llvm, llvm, replace);
Expand Down Expand Up @@ -1210,6 +1221,20 @@ impl Config {
toml::from_str(&contents)
.and_then(|table: toml::Value| TomlConfig::deserialize(table))
.unwrap_or_else(|err| {
if let Ok(Some(changes)) = toml::from_str(&contents)
.and_then(|table: toml::Value| ChangeIdWrapper::deserialize(table))
.and_then(|change_id| {
Ok(change_id.inner.map(|id| crate::find_recent_config_change_ids(id)))
})
{
if !changes.is_empty() {
println!(
"WARNING: There have been changes to x.py since you last updated:\n{}",
crate::human_readable_changes(&changes)
);
}
}

eprintln!("failed to parse TOML configuration '{}': {err}", file.display());
exit!(2);
})
Expand Down Expand Up @@ -1376,7 +1401,7 @@ impl Config {
toml.merge(override_toml, ReplaceOpt::Override);

config.changelog_seen = toml.changelog_seen;
config.change_id = toml.change_id;
config.change_id = toml.change_id.inner;

let Build {
build,
Expand Down
19 changes: 18 additions & 1 deletion src/bootstrap/src/core/config/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{flags::Flags, Config};
use super::{flags::Flags, ChangeIdWrapper, Config};
use crate::core::config::{LldMode, TomlConfig};

use clap::CommandFactory;
Expand Down Expand Up @@ -237,3 +237,20 @@ fn rust_lld() {
assert!(matches!(parse("rust.use-lld = true").lld_mode, LldMode::External));
assert!(matches!(parse("rust.use-lld = false").lld_mode, LldMode::Unused));
}

#[test]
#[should_panic]
fn parse_config_with_unknown_field() {
parse("unknown-key = 1");
}

#[test]
fn parse_change_id_with_unknown_field() {
let config = r#"
change-id = 3461
unknown-key = 1
"#;

let change_id_wrapper: ChangeIdWrapper = toml::from_str(config).unwrap();
assert_eq!(change_id_wrapper.inner, Some(3461));
}
4 changes: 3 additions & 1 deletion src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ mod utils;
pub use core::builder::PathSet;
pub use core::config::flags::Subcommand;
pub use core::config::Config;
pub use utils::change_tracker::{find_recent_config_change_ids, CONFIG_CHANGE_HISTORY};
pub use utils::change_tracker::{
find_recent_config_change_ids, human_readable_changes, CONFIG_CHANGE_HISTORY,
};

const LLVM_TOOLS: &[&str] = &[
"llvm-cov", // used to generate coverage report
Expand Down
14 changes: 14 additions & 0 deletions src/bootstrap/src/utils/change_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,20 @@ pub fn find_recent_config_change_ids(current_id: usize) -> Vec<ChangeInfo> {
.collect()
}

pub fn human_readable_changes(changes: &[ChangeInfo]) -> String {
let mut message = String::new();

for change in changes {
message.push_str(&format!(" [{}] {}\n", change.severity, change.summary));
message.push_str(&format!(
" - PR Link https://github.com/rust-lang/rust/pull/{}\n",
change.change_id
));
}

message
}

/// Keeps track of major changes made to the bootstrap configuration.
///
/// If you make any major changes (such as adding new values or changing default values),
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/suggestions/suppress-consider-slicing-issue-120605.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
pub struct Struct {
a: Vec<Struct>,
}

impl Struct {
pub fn test(&self) {
if let [Struct { a: [] }] = &self.a {
//~^ ERROR expected an array or slice
//~| ERROR expected an array or slice
println!("matches!")
}

if let [Struct { a: [] }] = &self.a[..] {
//~^ ERROR expected an array or slice
println!("matches!")
}
}
}

fn main() {}
23 changes: 23 additions & 0 deletions tests/ui/suggestions/suppress-consider-slicing-issue-120605.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error[E0529]: expected an array or slice, found `Vec<Struct>`
--> $DIR/suppress-consider-slicing-issue-120605.rs:7:16
|
LL | if let [Struct { a: [] }] = &self.a {
| ^^^^^^^^^^^^^^^^^^ ------- help: consider slicing here: `&self.a[..]`
| |
| pattern cannot match with input type `Vec<Struct>`

error[E0529]: expected an array or slice, found `Vec<Struct>`
--> $DIR/suppress-consider-slicing-issue-120605.rs:7:29
|
LL | if let [Struct { a: [] }] = &self.a {
| ^^ pattern cannot match with input type `Vec<Struct>`

error[E0529]: expected an array or slice, found `Vec<Struct>`
--> $DIR/suppress-consider-slicing-issue-120605.rs:13:29
|
LL | if let [Struct { a: [] }] = &self.a[..] {
| ^^ pattern cannot match with input type `Vec<Struct>`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0529`.

0 comments on commit 35dfc67

Please sign in to comment.