Skip to content

Commit 4bd4838

Browse files
oli-obkoli-obk
committed
Implement strict comment parsing for ui tests
1 parent cde87d1 commit 4bd4838

File tree

3 files changed

+235
-32
lines changed

3 files changed

+235
-32
lines changed

‎miri

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,8 @@ test|test-debug|bless|bless-debug)
181181
esac
182182
# Then test, and let caller control flags.
183183
# Only in root project and ui_test as `cargo-miri` has no tests.
184-
$CARGO test $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@"
185184
$CARGO test $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/ui_test/Cargo.toml "$@"
185+
$CARGO test $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@"
186186
;;
187187
run|run-debug)
188188
# Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so

‎ui_test/src/comments.rs

Lines changed: 209 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,43 @@ impl Condition {
6464
}
6565
}
6666

67+
macro_rules! checked {
68+
($path:expr, $l:expr) => {
69+
let path = $path;
70+
let l = $l;
71+
#[allow(unused_macros)]
72+
macro_rules! exit {
73+
($fmt:expr $$(,$args:expr)*) => {{
74+
eprint!("{}:{l}: ", path.display());
75+
eprintln!($fmt, $$($args,)*);
76+
#[cfg(not(test))]
77+
std::process::exit(1);
78+
#[cfg(test)]
79+
panic!();
80+
}};
81+
}
82+
#[allow(unused_macros)]
83+
macro_rules! check {
84+
($cond:expr, $fmt:expr $$(,$args:expr)*) => {{
85+
if !$cond {
86+
exit!($fmt $$(,$args)*);
87+
}
88+
}};
89+
}
90+
#[allow(unused_macros)]
91+
macro_rules! unwrap {
92+
($cond:expr, $fmt:expr $$(,$args:expr)*) => {{
93+
match $cond {
94+
Some(val) => val,
95+
None => {
96+
exit!($fmt $$(,$args)*);
97+
}
98+
}
99+
}};
100+
}
101+
};
102+
}
103+
67104
impl Comments {
68105
pub(crate) fn parse_file(path: &Path) -> Self {
69106
let content = std::fs::read_to_string(path).unwrap();
@@ -72,14 +109,45 @@ impl Comments {
72109

73110
/// Parse comments in `content`.
74111
/// `path` is only used to emit diagnostics if parsing fails.
75-
pub(crate) fn parse(path: &Path, content: &str) -> Self {
112+
///
113+
/// This function will only parse `//@` and `//~` style comments
114+
/// and ignore all others
115+
fn parse_checked(path: &Path, content: &str) -> Self {
76116
let mut this = Self::default();
77-
let error_pattern_regex =
78-
Regex::new(r"//(\[(?P<revision>[^\]]+)\])?~(?P<offset>\||[\^]+)? *(?P<level>ERROR|HELP|WARN|NOTE)?:?(?P<text>.*)")
79-
.unwrap();
80117

81118
// The line that a `|` will refer to
82119
let mut fallthrough_to = None;
120+
for (l, line) in content.lines().enumerate() {
121+
let l = l + 1; // enumerate starts at 0, but line numbers start at 1
122+
if let Some((_, command)) = line.split_once("//@") {
123+
let command = command.trim();
124+
if let Some((command, args)) = command.split_once(':') {
125+
this.parse_command_with_args(command, args, path, l);
126+
} else if let Some((command, _comments)) = command.split_once(' ') {
127+
this.parse_command(command, path, l)
128+
} else {
129+
this.parse_command(command, path, l)
130+
}
131+
} else if let Some((_, pattern)) = line.split_once("//~") {
132+
this.parse_pattern(pattern, &mut fallthrough_to, path, l)
133+
} else if let Some((_, pattern)) = line.split_once("//[") {
134+
this.parse_revisioned_pattern(pattern, &mut fallthrough_to, path, l)
135+
} else {
136+
fallthrough_to = None;
137+
}
138+
}
139+
this
140+
}
141+
142+
/// Parse comments in `content`.
143+
/// `path` is only used to emit diagnostics if parsing fails.
144+
pub(crate) fn parse(path: &Path, content: &str) -> Self {
145+
let mut this = Self::parse_checked(path, content);
146+
if content.contains("//@") {
147+
// Migration mode: if new syntax is used, ignore all old syntax
148+
return this;
149+
}
150+
83151
for (l, line) in content.lines().enumerate() {
84152
let l = l + 1; // enumerate starts at 0, but line numbers start at 1
85153
if let Some(revisions) = line.strip_prefix("// revisions:") {
@@ -140,36 +208,146 @@ impl Comments {
140208
);
141209
this.error_pattern = Some((s.trim().to_string(), l));
142210
}
143-
if let Some(captures) = error_pattern_regex.captures(line) {
144-
// FIXME: check that the error happens on the marked line
145-
let matched = captures["text"].trim().to_string();
211+
}
212+
this
213+
}
146214

147-
let revision = captures.name("revision").map(|rev| rev.as_str().to_string());
215+
fn parse_command_with_args(&mut self, command: &str, args: &str, path: &Path, l: usize) {
216+
checked!(path, l);
217+
match command {
218+
"revisions" => {
219+
check!(self.revisions.is_none(), "cannot specifiy revisions twice");
220+
self.revisions = Some(args.split_whitespace().map(|s| s.to_string()).collect());
221+
}
222+
"compile-flags" => {
223+
self.compile_flags.extend(args.split_whitespace().map(|s| s.to_string()));
224+
}
225+
"rustc-env" =>
226+
for env in args.split_whitespace() {
227+
let (k, v) = unwrap!(
228+
env.split_once('='),
229+
"environment variables must be key/value pairs separated by a `=`"
230+
);
231+
self.env_vars.push((k.to_string(), v.to_string()));
232+
},
233+
"normalize-stderr-test" => {
234+
let (from, to) =
235+
unwrap!(args.split_once("->"), "normalize-stderr-test needs a `->`");
236+
let from = from.trim().trim_matches('"');
237+
let to = to.trim().trim_matches('"');
238+
let from = unwrap!(Regex::new(from).ok(), "invalid regex");
239+
self.normalize_stderr.push((from, to.to_string()));
240+
}
241+
"error-pattern" => {
242+
check!(
243+
self.error_pattern.is_none(),
244+
"cannot specifiy error_pattern twice, previous: {:?}",
245+
self.error_pattern
246+
);
247+
self.error_pattern = Some((args.trim().to_string(), l));
248+
}
249+
_ => exit!("unknown command {command} with args {args}"),
250+
}
251+
}
148252

149-
let level = captures.name("level").map(|rev| rev.as_str().parse().unwrap());
253+
fn parse_command(&mut self, command: &str, path: &Path, l: usize) {
254+
checked!(path, l);
150255

151-
let match_line = match captures.name("offset").map(|rev| rev.as_str()) {
152-
Some("|") => fallthrough_to.expect("`//~|` pattern without preceding line"),
153-
Some(pat) => {
154-
debug_assert!(pat.chars().all(|c| c == '^'));
155-
l - pat.len()
156-
}
157-
None => l,
158-
};
159-
160-
fallthrough_to = Some(match_line);
161-
162-
this.error_matches.push(ErrorMatch {
163-
matched,
164-
revision,
165-
level,
166-
definition_line: l,
167-
line: match_line,
168-
});
169-
} else {
170-
fallthrough_to = None;
171-
}
256+
if let Some(s) = command.strip_prefix("ignore-") {
257+
self.ignore.push(Condition::parse(s));
258+
return;
172259
}
173-
this
260+
261+
if let Some(s) = command.strip_prefix("only-") {
262+
self.only.push(Condition::parse(s));
263+
return;
264+
}
265+
266+
if command.starts_with("stderr-per-bitwidth") {
267+
check!(!self.stderr_per_bitwidth, "cannot specifiy stderr-per-bitwidth twice");
268+
self.stderr_per_bitwidth = true;
269+
return;
270+
}
271+
272+
exit!("unknown command {command}");
273+
}
274+
275+
fn parse_pattern(
276+
&mut self,
277+
pattern: &str,
278+
fallthrough_to: &mut Option<usize>,
279+
path: &Path,
280+
l: usize,
281+
) {
282+
self.parse_pattern_inner(pattern, fallthrough_to, None, path, l)
283+
}
284+
285+
fn parse_revisioned_pattern(
286+
&mut self,
287+
pattern: &str,
288+
fallthrough_to: &mut Option<usize>,
289+
path: &Path,
290+
l: usize,
291+
) {
292+
checked!(path, l);
293+
let (revision, pattern) =
294+
unwrap!(pattern.split_once(']'), "`//[` without corresponding `]`");
295+
if pattern.starts_with('~') {
296+
self.parse_pattern_inner(
297+
&pattern[1..],
298+
fallthrough_to,
299+
Some(revision.to_owned()),
300+
path,
301+
l,
302+
)
303+
} else {
304+
exit!("revisioned pattern must have `~` following the `]`");
305+
}
306+
}
307+
308+
// parse something like (?P<offset>\||[\^]+)? *(?P<level>ERROR|HELP|WARN|NOTE)?:?(?P<text>.*)
309+
fn parse_pattern_inner(
310+
&mut self,
311+
pattern: &str,
312+
fallthrough_to: &mut Option<usize>,
313+
revision: Option<String>,
314+
path: &Path,
315+
l: usize,
316+
) {
317+
checked!(path, l);
318+
// FIXME: check that the error happens on the marked line
319+
320+
let (match_line, pattern) = match unwrap!(pattern.chars().next(), "no pattern specified") {
321+
'|' =>
322+
(*unwrap!(fallthrough_to, "`//~|` pattern without preceding line"), &pattern[1..]),
323+
'^' => {
324+
let offset = pattern.chars().take_while(|&c| c == '^').count();
325+
(l - offset, &pattern[offset..])
326+
}
327+
_ => (l, pattern),
328+
};
329+
330+
let (level, pattern) = match pattern.trim_start().split_once(|c| matches!(c, ':' | ' ')) {
331+
None => (None, pattern),
332+
Some((level, pattern_without_level)) =>
333+
match level.parse().ok() {
334+
Some(level) => (Some(level), pattern_without_level),
335+
None => (None, pattern),
336+
},
337+
};
338+
339+
let matched = pattern.trim().to_string();
340+
341+
check!(!matched.is_empty(), "no pattern specified");
342+
343+
*fallthrough_to = Some(match_line);
344+
345+
self.error_matches.push(ErrorMatch {
346+
matched,
347+
revision,
348+
level,
349+
definition_line: l,
350+
line: match_line,
351+
});
174352
}
175353
}

‎ui_test/src/comments/tests.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,28 @@ fn main() {
2020
"encountered a dangling reference (address $HEX is unallocated)"
2121
);
2222
}
23+
24+
#[test]
25+
fn parse_slash_slash_at() {
26+
let s = r"
27+
//@ error-pattern: foomp
28+
use std::mem;
29+
30+
";
31+
let comments = Comments::parse(Path::new("<dummy>"), s);
32+
println!("parsed comments: {:#?}", comments);
33+
assert_eq!(comments.error_pattern, Some(("foomp".to_string(), 2)));
34+
}
35+
36+
#[test]
37+
#[should_panic]
38+
fn parse_slash_slash_at_fail() {
39+
let s = r"
40+
//@ error-pattern foomp
41+
use std::mem;
42+
43+
";
44+
let comments = Comments::parse(Path::new("<dummy>"), s);
45+
println!("parsed comments: {:#?}", comments);
46+
assert_eq!(comments.error_pattern, Some(("foomp".to_string(), 2)));
47+
}

0 commit comments

Comments
 (0)