Skip to content

Commit b533f6c

Browse files
rust-timerrust-timer
authored
Unrolled build for rust-lang#126370
Rollup merge of rust-lang#126370 - Zalathar:normalize, r=oli-obk compiletest: Stricter parsing of `//@ normalize-*` headers I noticed some problems with the existing parser for these headers: - It is extremely lax, and basically ignores everything other than the text between two pairs of double-quote characters. - Unlike other name-value headers, it doesn't even check for a colon after the header name, so the test suite contains a mixture of with-colon and without-colon normalization rules. - If parsing fails, the header is silently ignored. The latter is especially bad for platform-specific normalization rules, because the lack of normalization probably won't be noticed until the test mysteriously fails in one of the full CI jobs.
2 parents 921645c + e09eedb commit b533f6c

File tree

5 files changed

+74
-61
lines changed

5 files changed

+74
-61
lines changed

‎src/tools/compiletest/src/header.rs

+27-21
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use crate::common::{Config, Debugger, FailMode, Mode, PassMode};
1414
use crate::header::cfg::parse_cfg_name_directive;
1515
use crate::header::cfg::MatchOutcome;
1616
use crate::header::needs::CachedNeedsConditions;
17+
use crate::util::static_regex;
1718
use crate::{extract_cdb_version, extract_gdb_version};
1819

1920
mod cfg;
@@ -1186,11 +1187,11 @@ impl Config {
11861187
}
11871188
}
11881189

1189-
fn parse_custom_normalization(&self, mut line: &str, prefix: &str) -> Option<(String, String)> {
1190+
fn parse_custom_normalization(&self, line: &str, prefix: &str) -> Option<(String, String)> {
11901191
if parse_cfg_name_directive(self, line, prefix).outcome == MatchOutcome::Match {
1191-
let from = parse_normalization_string(&mut line)?;
1192-
let to = parse_normalization_string(&mut line)?;
1193-
Some((from, to))
1192+
let (regex, replacement) = parse_normalize_rule(line)
1193+
.unwrap_or_else(|| panic!("couldn't parse custom normalization rule: `{line}`"));
1194+
Some((regex, replacement))
11941195
} else {
11951196
None
11961197
}
@@ -1311,24 +1312,29 @@ fn expand_variables(mut value: String, config: &Config) -> String {
13111312
value
13121313
}
13131314

1314-
/// Finds the next quoted string `"..."` in `line`, and extract the content from it. Move the `line`
1315-
/// variable after the end of the quoted string.
1316-
///
1317-
/// # Examples
1318-
///
1319-
/// ```
1320-
/// let mut s = "normalize-stderr-32bit: \"something (32 bits)\" -> \"something ($WORD bits)\".";
1321-
/// let first = parse_normalization_string(&mut s);
1322-
/// assert_eq!(first, Some("something (32 bits)".to_owned()));
1323-
/// assert_eq!(s, " -> \"something ($WORD bits)\".");
1315+
/// Parses the regex and replacement values of a `//@ normalize-*` header,
1316+
/// in the format:
1317+
/// ```text
1318+
/// normalize-*: "REGEX" -> "REPLACEMENT"
13241319
/// ```
1325-
fn parse_normalization_string(line: &mut &str) -> Option<String> {
1326-
// FIXME support escapes in strings.
1327-
let begin = line.find('"')? + 1;
1328-
let end = line[begin..].find('"')? + begin;
1329-
let result = line[begin..end].to_owned();
1330-
*line = &line[end + 1..];
1331-
Some(result)
1320+
fn parse_normalize_rule(header: &str) -> Option<(String, String)> {
1321+
// FIXME(#126370): A colon after the header name should be mandatory, but
1322+
// currently is not, and there are many tests that lack the colon.
1323+
// FIXME: Support escaped double-quotes in strings.
1324+
let captures = static_regex!(
1325+
r#"(?x) # (verbose mode regex)
1326+
^
1327+
[^:\s]+:?\s* # (header name followed by optional colon)
1328+
"(?<regex>[^"]*)" # "REGEX"
1329+
\s+->\s+ # ->
1330+
"(?<replacement>[^"]*)" # "REPLACEMENT"
1331+
$
1332+
"#
1333+
)
1334+
.captures(header)?;
1335+
let regex = captures["regex"].to_owned();
1336+
let replacement = captures["replacement"].to_owned();
1337+
Some((regex, replacement))
13321338
}
13331339

13341340
pub fn extract_llvm_version(version: &str) -> Option<u32> {

‎src/tools/compiletest/src/header/tests.rs

+36-30
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::path::Path;
33
use std::str::FromStr;
44

55
use crate::common::{Config, Debugger, Mode};
6-
use crate::header::{parse_normalization_string, EarlyProps, HeadersCache};
6+
use crate::header::{parse_normalize_rule, EarlyProps, HeadersCache};
77

88
use super::iter_header;
99

@@ -32,35 +32,41 @@ fn make_test_description<R: Read>(
3232
}
3333

3434
#[test]
35-
fn test_parse_normalization_string() {
36-
let mut s = "normalize-stderr-32bit: \"something (32 bits)\" -> \"something ($WORD bits)\".";
37-
let first = parse_normalization_string(&mut s);
38-
assert_eq!(first, Some("something (32 bits)".to_owned()));
39-
assert_eq!(s, " -> \"something ($WORD bits)\".");
40-
41-
// Nothing to normalize (No quotes)
42-
let mut s = "normalize-stderr-32bit: something (32 bits) -> something ($WORD bits).";
43-
let first = parse_normalization_string(&mut s);
44-
assert_eq!(first, None);
45-
assert_eq!(s, r#"normalize-stderr-32bit: something (32 bits) -> something ($WORD bits)."#);
46-
47-
// Nothing to normalize (Only a single quote)
48-
let mut s = "normalize-stderr-32bit: \"something (32 bits) -> something ($WORD bits).";
49-
let first = parse_normalization_string(&mut s);
50-
assert_eq!(first, None);
51-
assert_eq!(s, "normalize-stderr-32bit: \"something (32 bits) -> something ($WORD bits).");
52-
53-
// Nothing to normalize (Three quotes)
54-
let mut s = "normalize-stderr-32bit: \"something (32 bits)\" -> \"something ($WORD bits).";
55-
let first = parse_normalization_string(&mut s);
56-
assert_eq!(first, Some("something (32 bits)".to_owned()));
57-
assert_eq!(s, " -> \"something ($WORD bits).");
58-
59-
// Nothing to normalize (No quotes, 16-bit)
60-
let mut s = "normalize-stderr-16bit: something (16 bits) -> something ($WORD bits).";
61-
let first = parse_normalization_string(&mut s);
62-
assert_eq!(first, None);
63-
assert_eq!(s, r#"normalize-stderr-16bit: something (16 bits) -> something ($WORD bits)."#);
35+
fn test_parse_normalize_rule() {
36+
let good_data = &[
37+
(
38+
r#"normalize-stderr-32bit: "something (32 bits)" -> "something ($WORD bits)""#,
39+
"something (32 bits)",
40+
"something ($WORD bits)",
41+
),
42+
// FIXME(#126370): A colon after the header name should be mandatory,
43+
// but currently is not, and there are many tests that lack the colon.
44+
(
45+
r#"normalize-stderr-32bit "something (32 bits)" -> "something ($WORD bits)""#,
46+
"something (32 bits)",
47+
"something ($WORD bits)",
48+
),
49+
];
50+
51+
for &(input, expected_regex, expected_replacement) in good_data {
52+
let parsed = parse_normalize_rule(input);
53+
let parsed =
54+
parsed.as_ref().map(|(regex, replacement)| (regex.as_str(), replacement.as_str()));
55+
assert_eq!(parsed, Some((expected_regex, expected_replacement)));
56+
}
57+
58+
let bad_data = &[
59+
r#"normalize-stderr-16bit: something (16 bits) -> something ($WORD bits)"#,
60+
r#"normalize-stderr-32bit: something (32 bits) -> something ($WORD bits)"#,
61+
r#"normalize-stderr-32bit: "something (32 bits) -> something ($WORD bits)"#,
62+
r#"normalize-stderr-32bit: "something (32 bits)" -> "something ($WORD bits)"#,
63+
r#"normalize-stderr-32bit: "something (32 bits)" -> "something ($WORD bits)"."#,
64+
];
65+
66+
for &input in bad_data {
67+
let parsed = parse_normalize_rule(input);
68+
assert_eq!(parsed, None);
69+
}
6470
}
6571

6672
#[derive(Default)]

‎src/tools/compiletest/src/runtest.rs

+1-9
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use crate::errors::{self, Error, ErrorKind};
1515
use crate::header::TestProps;
1616
use crate::json;
1717
use crate::read2::{read2_abbreviated, Truncated};
18-
use crate::util::{add_dylib_path, copy_dir_all, dylib_env_var, logv, PathBufExt};
18+
use crate::util::{add_dylib_path, copy_dir_all, dylib_env_var, logv, static_regex, PathBufExt};
1919
use crate::ColorConfig;
2020
use colored::Colorize;
2121
use miropt_test_tools::{files_for_miropt_test, MiroptTest, MiroptTestFile};
@@ -48,14 +48,6 @@ use debugger::DebuggerCommands;
4848
#[cfg(test)]
4949
mod tests;
5050

51-
macro_rules! static_regex {
52-
($re:literal) => {{
53-
static RE: ::std::sync::OnceLock<::regex::Regex> = ::std::sync::OnceLock::new();
54-
RE.get_or_init(|| ::regex::Regex::new($re).unwrap())
55-
}};
56-
}
57-
use static_regex;
58-
5951
const FAKE_SRC_BASE: &str = "fake-test-src-base";
6052

6153
#[cfg(windows)]

‎src/tools/compiletest/src/runtest/coverage.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ use std::process::Command;
77
use glob::glob;
88

99
use crate::common::{UI_COVERAGE, UI_COVERAGE_MAP};
10-
use crate::runtest::{static_regex, Emit, ProcRes, TestCx, WillExecute};
10+
use crate::runtest::{Emit, ProcRes, TestCx, WillExecute};
11+
use crate::util::static_regex;
1112

1213
impl<'test> TestCx<'test> {
1314
fn coverage_dump_path(&self) -> &Path {

‎src/tools/compiletest/src/util.rs

+8
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,11 @@ pub fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> std::io::Re
9090
}
9191
Ok(())
9292
}
93+
94+
macro_rules! static_regex {
95+
($re:literal) => {{
96+
static RE: ::std::sync::OnceLock<::regex::Regex> = ::std::sync::OnceLock::new();
97+
RE.get_or_init(|| ::regex::Regex::new($re).unwrap())
98+
}};
99+
}
100+
pub(crate) use static_regex;

0 commit comments

Comments
 (0)