compiletest silently fails on a name-value directive with a known name but missing colon but does not report an error

[jieyouxu]

compiletest expects that a name-value directive, like //@ revisions: foo, to have the colon :

rust/src/tools/compiletest/src/header.rs

Lines 1137 to 1146 in b14d8b2

	pub fn parse_name_value_directive(&self, line: &str, directive: &str) -> Option<String> {
	let colon = directive.len();
	if line.starts_with(directive) && line.as_bytes().get(colon) == Some(&b':') {
	let value = line[(colon + 1)..].to_owned();
	debug!("{}: {}", directive, value);
	Some(expand_variables(value, self))
	} else {
	None
	}
	}

If the name-value directive contains a known directive name like revisions but does not have the colon (i.e. //@ revisions foo), then:

• compiletest known directive check accepts the known directive name revisions
• parse_name_value_directive expects revisions: but only got revisions, so parsing fails
• no other compiletest directive parsing rules accept revisions directive name
• no errors raised yet there is no effect (no revisions in this example).

compiletest should not silently fail here because it's very surprising and a pain to debug unless you know exactly what's wrong.

[jieyouxu]

I think an actual ui test has this problem?

rust/tests/ui/precondition-checks/cfg-ub-checks-default.rs

Line 2 in b14d8b2

//@ revisions YES NO

[jieyouxu]

Wait if I change it to //@ revisions: YES NO which actually enables the compile-flags directives, this test still passes. What is this test actually checking then? Maybe it just doesn't actually need the compile-flags?

[fmease]

Wait if I change it to //@ revisions: YES NO which actually enables the compile-flags directives, this test still passes. What is this test actually checking then? Maybe it just doesn't actually need the compile-flags?

Well, the test “needs” the compile-flags because it's checking that cfg ub_checks is equivalent to cfg debug_assertions for all values of debug_assertions, i.e., yes and no (under the assumption that -Zub-checks wasn't passed which holds). If you remove the flags the test covers fewer cases.

cc #123411 &

rust/compiler/rustc_session/src/options.rs

Line 1997 in b3bd705

"emit runtime checks for Undefined Behavior (default: -Cdebug-assertions)"),

[jieyouxu]

Yeah, I talked to Saethlin, makes sense. It's just compiletest silently failing here is awful 😆

[jieyouxu]

I briefly looked at this, the easy fix is trivial, but the error handling is entirely not maintainable and involves propagating a poisoned: &mut bool status bool through many callsites. The easy fix is like

I think the directive parsing is long past due for a rework. The error handling needs to not rely on passing a &mut bool around everywhere.