Skip to content

Commit 1cf3fa8

Browse files
authored
Unrolled build for rust-lang#127457
Rollup merge of rust-lang#127457 - donno2048:master, r=albertlarsan68 Make tidy fast without compromising case alternation Fixes tidy speed issue but still catches case-alternation, enabled for other `style.rs` files, and also detects test files better. r? `@albertlarsan68` `@Nilstrieb`
2 parents 2ccafed + 23e346e commit 1cf3fa8

File tree

2 files changed

+35
-31
lines changed

2 files changed

+35
-31
lines changed

src/tools/tidy/src/style.rs

+29-18
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818
// ignore-tidy-dbg
1919

2020
use crate::walk::{filter_dirs, walk};
21-
use regex::RegexSet;
21+
use regex::RegexSetBuilder;
2222
use rustc_hash::FxHashMap;
23-
use std::{ffi::OsStr, path::Path};
23+
use std::{ffi::OsStr, path::Path, sync::LazyLock};
2424

2525
#[cfg(test)]
2626
mod tests;
@@ -110,16 +110,26 @@ const ROOT_PROBLEMATIC_CONSTS: &[u32] = &[
110110
173390526, 721077,
111111
];
112112

113+
const LETTER_DIGIT: &[(char, char)] = &[('A', '4'), ('B', '8'), ('E', '3')];
114+
113115
// Returns all permutations of problematic consts, over 2000 elements.
114116
fn generate_problematic_strings(
115117
consts: &[u32],
116118
letter_digit: &FxHashMap<char, char>,
117119
) -> Vec<String> {
118120
generate_problems(consts, letter_digit)
119-
.flat_map(|v| vec![v.to_string(), format!("{:x}", v), format!("{:X}", v)])
121+
.flat_map(|v| vec![v.to_string(), format!("{:X}", v)])
120122
.collect()
121123
}
122124

125+
static PROBLEMATIC_CONSTS_STRINGS: LazyLock<Vec<String>> = LazyLock::new(|| {
126+
generate_problematic_strings(ROOT_PROBLEMATIC_CONSTS, &LETTER_DIGIT.iter().cloned().collect())
127+
});
128+
129+
fn contains_problematic_const(trimmed: &str) -> bool {
130+
PROBLEMATIC_CONSTS_STRINGS.iter().any(|s| trimmed.to_uppercase().contains(s))
131+
}
132+
123133
const INTERNAL_COMPILER_DOCS_LINE: &str = "#### This error code is internal to the compiler and will not be emitted with normal Rust code.";
124134

125135
/// Parser states for `line_is_url`.
@@ -316,14 +326,14 @@ pub fn check(path: &Path, bad: &mut bool) {
316326
// We only check CSS files in rustdoc.
317327
path.extension().map_or(false, |e| e == "css") && !is_in(path, "src", "librustdoc")
318328
}
319-
let problematic_consts_strings = generate_problematic_strings(
320-
ROOT_PROBLEMATIC_CONSTS,
321-
&[('A', '4'), ('B', '8'), ('E', '3')].iter().cloned().collect(),
322-
);
329+
323330
// This creates a RegexSet as regex contains performance optimizations to be able to deal with these over
324331
// 2000 needles efficiently. This runs over the entire source code, so performance matters.
325-
let problematic_regex = RegexSet::new(problematic_consts_strings.as_slice()).unwrap();
326-
332+
let problematic_regex = RegexSetBuilder::new(PROBLEMATIC_CONSTS_STRINGS.as_slice())
333+
.case_insensitive(true)
334+
.build()
335+
.unwrap();
336+
let style_file = Path::new(file!());
327337
walk(path, skip, &mut |entry, contents| {
328338
let file = entry.path();
329339
let filename = file.file_name().unwrap().to_string_lossy();
@@ -389,10 +399,15 @@ pub fn check(path: &Path, bad: &mut bool) {
389399
let mut lines = 0;
390400
let mut last_safety_comment = false;
391401
let mut comment_block: Option<(usize, usize)> = None;
392-
let is_test = file.components().any(|c| c.as_os_str() == "tests");
402+
let is_test = file.components().any(|c| c.as_os_str() == "tests")
403+
|| file.file_stem().unwrap() == "tests";
404+
let is_style = file.ends_with(style_file) || style_file.ends_with(file);
405+
let is_style_test =
406+
is_test && file.parent().unwrap().ends_with(style_file.with_extension(""));
393407
// scanning the whole file for multiple needles at once is more efficient than
394408
// executing lines times needles separate searches.
395-
let any_problematic_line = problematic_regex.is_match(contents);
409+
let any_problematic_line =
410+
!is_style && !is_style_test && problematic_regex.is_match(contents);
396411
for (i, line) in contents.split('\n').enumerate() {
397412
if line.is_empty() {
398413
if i == 0 {
@@ -451,7 +466,7 @@ pub fn check(path: &Path, bad: &mut bool) {
451466
if line.contains('\r') {
452467
suppressible_tidy_err!(err, skip_cr, "CR character");
453468
}
454-
if filename != "style.rs" {
469+
if !is_style {
455470
// Allow using TODO in diagnostic suggestions by marking the
456471
// relevant line with `// ignore-tidy-todo`.
457472
if trimmed.contains("TODO") && !trimmed.contains("ignore-tidy-todo") {
@@ -462,12 +477,8 @@ pub fn check(path: &Path, bad: &mut bool) {
462477
if trimmed.contains("//") && trimmed.contains(" XXX") {
463478
err("Instead of XXX use FIXME")
464479
}
465-
if any_problematic_line {
466-
for s in problematic_consts_strings.iter() {
467-
if trimmed.contains(s) {
468-
err("Don't use magic numbers that spell things (consider 0x12345678)");
469-
}
470-
}
480+
if any_problematic_line && contains_problematic_const(trimmed) {
481+
err("Don't use magic numbers that spell things (consider 0x12345678)");
471482
}
472483
}
473484
// for now we just check libcore

src/tools/tidy/src/style/tests.rs

+6-13
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,10 @@
11
use super::*;
22

33
#[test]
4-
fn test_generate_problematic_strings() {
5-
let problematic_regex = RegexSet::new(
6-
generate_problematic_strings(
7-
ROOT_PROBLEMATIC_CONSTS,
8-
&[('A', '4'), ('B', '8'), ('E', '3'), ('0', 'F')].iter().cloned().collect(), // use "futile" F intentionally
9-
)
10-
.as_slice(),
11-
)
12-
.unwrap();
13-
assert!(problematic_regex.is_match("786357")); // check with no "decimal" hex digits - converted to integer
14-
assert!(problematic_regex.is_match("589701")); // check with "decimal" replacements - converted to integer
15-
assert!(problematic_regex.is_match("8FF85")); // check for hex display
16-
assert!(!problematic_regex.is_match("1193046")); // check for non-matching value
4+
fn test_contains_problematic_const() {
5+
assert!(contains_problematic_const("721077")); // check with no "decimal" hex digits - converted to integer
6+
assert!(contains_problematic_const("524421")); // check with "decimal" replacements - converted to integer
7+
assert!(contains_problematic_const(&(285 * 281).to_string())); // check for hex display
8+
assert!(contains_problematic_const(&format!("{:x}B5", 2816))); // check for case-alternating hex display
9+
assert!(!contains_problematic_const("1193046")); // check for non-matching value
1710
}

0 commit comments

Comments
 (0)