diff --git a/src/parse/macros/cfg_select.rs b/src/parse/macros/cfg_select.rs index de8f61d2412..234070105e6 100644 --- a/src/parse/macros/cfg_select.rs +++ b/src/parse/macros/cfg_select.rs @@ -17,15 +17,22 @@ use crate::parse::macros::build_stream_parser; use crate::parse::session::ParseSess; use crate::spanned::Spanned; +// NB: parser recovery can return `Ok(expr)` (or other non-terminals) and still emit errors. We need +// to properly reset the shared error count. + pub(crate) fn parse_items_from_cfg_select<'a>( psess: &'a ParseSess, mac: &'a ast::MacCall, ) -> Result, &'static str> { - match catch_unwind(AssertUnwindSafe(|| { + let result = catch_unwind(AssertUnwindSafe(|| { parse_items_from_cfg_select_inner(psess, mac) - })) { - Ok(Ok(items)) => Ok(items), - Ok(err @ Err(_)) => err, + })); + let dcx = psess.inner().dcx(); + let recovered = dcx.has_errors().is_some(); + dcx.reset_err_count(); + match result { + Ok(Ok(_)) if recovered => Err("cfg_select! parsed with recovery"), + Ok(res) => res, Err(..) => Err("failed to parse cfg_select!"), } } @@ -71,7 +78,6 @@ fn parse_items_from_cfg_select_inner<'a>( } Err(err) => { err.cancel(); - parser.psess.dcx().reset_err_count(); return Err( "Expected item inside cfg_select block, but failed to parse it as an item", ); @@ -159,6 +165,24 @@ pub(crate) fn parse_cfg_select_arms( psess: &ParseSess, ts: TokenStream, ) -> Option> { + let result = catch_unwind(AssertUnwindSafe(|| parse_cfg_select_arms_inner(psess, ts))); + let dcx = psess.inner().dcx(); + let recovered = dcx.has_errors().is_some(); + dcx.reset_err_count(); + match result { + Ok(Some(_)) if recovered => { + debug!("cfg_select! arms parsed with recovery"); + None + } + Ok(arms) => arms, + Err(..) => { + debug!("failed to parse cfg_select! arms"); + None + } + } +} + +fn parse_cfg_select_arms_inner(psess: &ParseSess, ts: TokenStream) -> Option> { let mut cfg_select_predicates = vec![]; let mut parser = build_stream_parser(psess.inner(), ts); @@ -170,12 +194,13 @@ pub(crate) fn parse_cfg_select_arms( debug!("Failed to parse cfg entry in cfg_select! predicate"); return None; }; + CfgSelectFormatPredicate::Cfg(meta_item) }; if let Err(e) = parser.expect(exp!(FatArrow)) { e.cancel(); - debug!("Expected to find a `=>` after cfg_selec! predicate."); + debug!("Expected to find a `=>` after cfg_select! predicate."); return None; }; diff --git a/tests/rustfmt/main.rs b/tests/rustfmt/main.rs index 2220df2b617..5fbf64d8862 100644 --- a/tests/rustfmt/main.rs +++ b/tests/rustfmt/main.rs @@ -3,16 +3,19 @@ use indoc::panicdoc; use std::env; use std::fs::{File, remove_file}; +use std::io::Write; use std::path::Path; -use std::process::Command; +use std::process::{Command, Stdio}; use rustfmt_config_proc_macro::{nightly_only_test, rustfmt_only_ci_test, stable_only_test}; /// Run the rustfmt executable with environment vars set and return its output. +/// If `input` is given, it is piped to the executable's stdin. fn rustfmt_with_extra( args: &[&str], working_dir: Option<&str>, envs: &[(&str, &str)], + input: Option<&str>, ) -> (String, String) { let rustfmt_exe = env!("CARGO_BIN_EXE_rustfmt"); let bin_dir = Path::new(rustfmt_exe).parent().unwrap(); @@ -29,7 +32,19 @@ fn rustfmt_with_extra( if let Some(working_dir) = working_dir { cmd.current_dir(working_dir); } - match cmd.output() { + cmd.stdin(if input.is_some() { + Stdio::piped() + } else { + Stdio::null() + }); + cmd.stdout(Stdio::piped()); + cmd.stderr(Stdio::piped()); + match cmd.spawn().and_then(|mut child| { + if let Some(input) = input { + child.stdin.take().unwrap().write_all(input.as_bytes())?; + } + child.wait_with_output() + }) { Ok(output) => ( String::from_utf8(output.stdout).expect("utf-8"), String::from_utf8(output.stderr).expect("utf-8"), @@ -39,7 +54,7 @@ fn rustfmt_with_extra( } fn rustfmt(args: &[&str]) -> (String, String) { - rustfmt_with_extra(args, None, &[]) + rustfmt_with_extra(args, None, &[], None) } macro_rules! assert_that { @@ -401,9 +416,22 @@ fn rustfmt_allow_not_a_dir_errors() { let args = [empty_rs.to_str().unwrap()]; let envs = &[("HOME", fake_home_str)]; - let (stdout, stderr) = rustfmt_with_extra(&args, Some(fake_home_str), envs); + let (stdout, stderr) = rustfmt_with_extra(&args, Some(fake_home_str), envs, None); // Should pass without any errors assert_eq!(stdout, ""); assert_eq!(stderr, ""); } + +// FIXME(cfg_select): ungate when `cfg_select!` formatting can be enabled on stable. +#[nightly_only_test] +#[test] +fn cfg_select_parser_recovery_ice_7087() { + // See also https://github.com/rust-lang/rustfmt/issues/7087 + let source = "cfg_select! { true => { fn foo {} } }\n"; + let panic_re = regex::Regex::new("thread.*panicked").unwrap(); + let (stdout, stderr) = rustfmt_with_extra(&[], None, &[], Some(source)); + assert!(!panic_re.is_match(&stderr)); + // Invalid arm should cause `cfg_select!` to be re-emitted unmodified. + assert_eq!(source, stdout); +} diff --git a/tests/source/cfg-select-valid-then-invalid-item-error-count-leak.rs b/tests/source/cfg-select-valid-then-invalid-item-error-count-leak.rs new file mode 100644 index 00000000000..399fcd45e09 --- /dev/null +++ b/tests/source/cfg-select-valid-then-invalid-item-error-count-leak.rs @@ -0,0 +1,6 @@ +//@ rustfmt-unstable: true +cfg_select! { + unix => { fn bar() {} } +} + +cfg_select! { unix => { fn foo {} } } diff --git a/tests/target/cfg-select-valid-then-invalid-item-error-count-leak.rs b/tests/target/cfg-select-valid-then-invalid-item-error-count-leak.rs new file mode 100644 index 00000000000..d4337d26120 --- /dev/null +++ b/tests/target/cfg-select-valid-then-invalid-item-error-count-leak.rs @@ -0,0 +1,8 @@ +//@ rustfmt-unstable: true +cfg_select! { + unix => { + fn bar() {} + } +} + +cfg_select! { unix => { fn foo {} } }