Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable --check-cfg by default in UI tests #124345

Merged
merged 3 commits into from
May 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ pub struct TestProps {
pub llvm_cov_flags: Vec<String>,
/// Extra flags to pass to LLVM's `filecheck` tool, in tests that use it.
pub filecheck_flags: Vec<String>,
/// Don't automatically insert any `--check-cfg` args
pub no_auto_check_cfg: bool,
}

mod directives {
Expand Down Expand Up @@ -249,6 +251,7 @@ mod directives {
pub const COMPARE_OUTPUT_LINES_BY_SUBSET: &'static str = "compare-output-lines-by-subset";
pub const LLVM_COV_FLAGS: &'static str = "llvm-cov-flags";
pub const FILECHECK_FLAGS: &'static str = "filecheck-flags";
pub const NO_AUTO_CHECK_CFG: &'static str = "no-auto-check-cfg";
Urgau marked this conversation as resolved.
Show resolved Hide resolved
// This isn't a real directive, just one that is probably mistyped often
pub const INCORRECT_COMPILER_FLAGS: &'static str = "compiler-flags";
}
Expand Down Expand Up @@ -304,6 +307,7 @@ impl TestProps {
remap_src_base: false,
llvm_cov_flags: vec![],
filecheck_flags: vec![],
no_auto_check_cfg: false,
}
}

Expand Down Expand Up @@ -567,6 +571,8 @@ impl TestProps {
if let Some(flags) = config.parse_name_value_directive(ln, FILECHECK_FLAGS) {
self.filecheck_flags.extend(split_flags(&flags));
}

config.set_name_directive(ln, NO_AUTO_CHECK_CFG, &mut self.no_auto_check_cfg);
},
);

Expand Down Expand Up @@ -860,6 +866,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"needs-unwind",
"needs-wasmtime",
"needs-xray",
"no-auto-check-cfg",
"no-prefer-dynamic",
"normalize-stderr-32bit",
"normalize-stderr-64bit",
Expand Down
25 changes: 22 additions & 3 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1028,12 +1028,31 @@ impl<'test> TestCx<'test> {
}

fn set_revision_flags(&self, cmd: &mut Command) {
// Normalize revisions to be lowercase and replace `-`s with `_`s.
// Otherwise the `--cfg` flag is not valid.
let normalize_revision = |revision: &str| revision.to_lowercase().replace("-", "_");
Urgau marked this conversation as resolved.
Show resolved Hide resolved

if let Some(revision) = self.revision {
// Normalize revisions to be lowercase and replace `-`s with `_`s.
// Otherwise the `--cfg` flag is not valid.
let normalized_revision = revision.to_lowercase().replace("-", "_");
let normalized_revision = normalize_revision(revision);
cmd.args(&["--cfg", &normalized_revision]);
}

if !self.props.no_auto_check_cfg {
let mut check_cfg = String::with_capacity(25);

// Generate `cfg(FALSE, REV1, ..., REVN)` (for all possible revisions)
//
// For compatibility reason we consider the `FALSE` cfg to be expected
// since it is extensively used in the testsuite.
check_cfg.push_str("cfg(FALSE");
Comment on lines +1043 to +1047
Copy link
Member

@jieyouxu jieyouxu Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any documentation for this special case? If not, could you please document this behavior in the dev-guide so that the knowledge of FALSE being "special" exists somewhere? Or rather, if it's recommended to be used stylistically for configuring something out, can that be documented too?

Copy link
Member Author

@Urgau Urgau Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware, nor was I able to find any documentation regarding FALSE being a de-facto cfg that will eval to false.

could you please document this behavior in the dev-guide

Sure, I will send a PR for that.

EDIT: forget to send it, sorry

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rustc-dev-guide PR is up rust-lang/rustc-dev-guide#1966

for revision in &self.props.revisions {
check_cfg.push_str(",");
check_cfg.push_str(&normalize_revision(&revision));
}
check_cfg.push_str(")");

cmd.args(&["--check-cfg", &check_cfg]);
}
}

fn typecheck_source(&self, src: String) -> ProcRes {
Expand Down
11 changes: 4 additions & 7 deletions tests/codegen/instrument-coverage/instrument-coverage-off.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
// Test that `-Cinstrument-coverage=off` does not add coverage instrumentation to LLVM IR.

//@ needs-profiler-support
//@ revisions: n no off false zero
//@ revisions: n no off false_ zero
//@ [n] compile-flags: -Cinstrument-coverage=n
//@ [no] compile-flags: -Cinstrument-coverage=no
//@ [off] compile-flags: -Cinstrument-coverage=off
//@ [false] compile-flags: -Cinstrument-coverage=false
//@ [false_] compile-flags: -Cinstrument-coverage=false
//@ [zero] compile-flags: -Cinstrument-coverage=0

// CHECK-NOT: __llvm_profile_filename
// CHECK-NOT: __llvm_coverage_mapping

#![crate_type="lib"]
#![crate_type = "lib"]

#[inline(never)]
fn some_function() {

}
fn some_function() {}

pub fn some_other_function() {
some_function();
Expand Down
4 changes: 2 additions & 2 deletions tests/codegen/instrument-coverage/instrument-coverage.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// Test that `-Cinstrument-coverage` creates expected __llvm_profile_filename symbol in LLVM IR.

//@ needs-profiler-support
//@ revisions: default y yes on true all
//@ revisions: default y yes on true_ all
//@ [default] compile-flags: -Cinstrument-coverage
//@ [y] compile-flags: -Cinstrument-coverage=y
//@ [yes] compile-flags: -Cinstrument-coverage=yes
//@ [on] compile-flags: -Cinstrument-coverage=on
//@ [true] compile-flags: -Cinstrument-coverage=true
//@ [true_] compile-flags: -Cinstrument-coverage=true
//@ [all] compile-flags: -Cinstrument-coverage=all

// CHECK: @__llvm_profile_filename = {{.*}}"default_%m_%p.profraw\00"{{.*}}
Expand Down
6 changes: 3 additions & 3 deletions tests/rustdoc-ui/argfile/commandline-argfile.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// Check to see if we can get parameters from an @argsfile file
//
//@ check-pass
//@ compile-flags: --cfg cmdline_set @{{src-base}}/argfile/commandline-argfile.args
//@ compile-flags: --cfg cmdline_set --check-cfg=cfg(cmdline_set,unbroken)
//@ compile-flags: @{{src-base}}/argfile/commandline-argfile.args

#[cfg(not(cmdline_set))]
compile_error!("cmdline_set not set");

#[cfg(not(unbroken))]
compile_error!("unbroken not set");

fn main() {
}
fn main() {}
6 changes: 3 additions & 3 deletions tests/ui/argfile/commandline-argfile.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// Check to see if we can get parameters from an @argsfile file
//
//@ build-pass
//@ compile-flags: --cfg cmdline_set @{{src-base}}/argfile/commandline-argfile.args
//@ compile-flags: --cfg cmdline_set --check-cfg=cfg(cmdline_set,unbroken)
//@ compile-flags: @{{src-base}}/argfile/commandline-argfile.args

#[cfg(not(cmdline_set))]
compile_error!("cmdline_set not set");

#[cfg(not(unbroken))]
compile_error!("unbroken not set");

fn main() {
}
fn main() {}
3 changes: 2 additions & 1 deletion tests/ui/cfg/cfg-in-crate-1.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ run-pass
//@ compile-flags: --cfg bar -D warnings
//@ compile-flags: --cfg bar --check-cfg=cfg(bar) -D warnings

#![cfg(bar)]

fn main() {}
2 changes: 1 addition & 1 deletion tests/ui/cfg/cfg-macros-foo.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//@ run-pass
//@ compile-flags: --cfg foo
//@ compile-flags: --cfg foo --check-cfg=cfg(foo)

// check that cfg correctly chooses between the macro impls (see also
// cfg-macros-notfoo.rs)
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/cfg/cfg-path-error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//@ check-fail

#![allow(unexpected_cfgs)] // invalid cfgs

#[cfg(any(foo, foo::bar))]
//~^ERROR `cfg` predicate key must be an identifier
fn foo1() {}
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/cfg/cfg-path-error.stderr
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
error: `cfg` predicate key must be an identifier
--> $DIR/cfg-path-error.rs:3:16
--> $DIR/cfg-path-error.rs:5:16
|
LL | #[cfg(any(foo, foo::bar))]
| ^^^^^^^^

error: `cfg` predicate key must be an identifier
--> $DIR/cfg-path-error.rs:7:11
--> $DIR/cfg-path-error.rs:9:11
|
LL | #[cfg(any(foo::bar, foo))]
| ^^^^^^^^

error: `cfg` predicate key must be an identifier
--> $DIR/cfg-path-error.rs:11:16
--> $DIR/cfg-path-error.rs:13:16
|
LL | #[cfg(all(foo, foo::bar))]
| ^^^^^^^^

error: `cfg` predicate key must be an identifier
--> $DIR/cfg-path-error.rs:15:11
--> $DIR/cfg-path-error.rs:17:11
|
LL | #[cfg(all(foo::bar, foo))]
| ^^^^^^^^
Expand Down
4 changes: 3 additions & 1 deletion tests/ui/cfg/cfg_attr.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//@ run-pass
//@ compile-flags:--cfg set1 --cfg set2
#![allow(dead_code)]

#![allow(dead_code, unexpected_cfgs)]

use std::fmt::Debug;

struct NotDebugable;
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/cfg/cfgs-on-items.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
//@ run-pass
//@ compile-flags: --cfg fooA --cfg fooB
//@ compile-flags: --cfg fooA --cfg fooB --check-cfg=cfg(fooA,fooB,fooC,bar)

// fooA AND !bar

#[cfg(all(fooA, not(bar)))]
fn foo1() -> isize { 1 }

Expand Down
3 changes: 3 additions & 0 deletions tests/ui/cfg/diagnostics-not-a-def.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#![feature(lint_reasons)]

pub mod inner {
#[expect(unexpected_cfgs)]
pub fn i_am_here() {
#[cfg(feature = "another one that doesn't exist")]
loop {}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/cfg/diagnostics-not-a-def.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0425]: cannot find function `i_am_not` in module `inner`
--> $DIR/diagnostics-not-a-def.rs:11:12
--> $DIR/diagnostics-not-a-def.rs:14:12
|
LL | inner::i_am_not();
| ^^^^^^^^ not found in `inner`
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/cfg/diagnostics-same-crate.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(unexpected_cfgs)] // since we want to recognize them as unexpected

pub mod inner {
#[cfg(FALSE)]
pub fn uwu() {}
Expand Down
24 changes: 12 additions & 12 deletions tests/ui/cfg/diagnostics-same-crate.stderr
Original file line number Diff line number Diff line change
@@ -1,72 +1,72 @@
error[E0432]: unresolved import `super::inner::doesnt_exist`
--> $DIR/diagnostics-same-crate.rs:28:9
--> $DIR/diagnostics-same-crate.rs:30:9
|
LL | use super::inner::doesnt_exist;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ no `doesnt_exist` in `inner`
|
note: found an item that was configured out
--> $DIR/diagnostics-same-crate.rs:7:13
--> $DIR/diagnostics-same-crate.rs:9:13
|
LL | pub mod doesnt_exist {
| ^^^^^^^^^^^^

error[E0432]: unresolved import `super::inner::doesnt_exist`
--> $DIR/diagnostics-same-crate.rs:31:23
--> $DIR/diagnostics-same-crate.rs:33:23
|
LL | use super::inner::doesnt_exist::hi;
| ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner`
|
note: found an item that was configured out
--> $DIR/diagnostics-same-crate.rs:7:13
--> $DIR/diagnostics-same-crate.rs:9:13
|
LL | pub mod doesnt_exist {
| ^^^^^^^^^^^^

error[E0433]: failed to resolve: could not find `doesnt_exist` in `inner`
--> $DIR/diagnostics-same-crate.rs:50:12
--> $DIR/diagnostics-same-crate.rs:52:12
|
LL | inner::doesnt_exist::hello();
| ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner`
|
note: found an item that was configured out
--> $DIR/diagnostics-same-crate.rs:7:13
--> $DIR/diagnostics-same-crate.rs:9:13
|
LL | pub mod doesnt_exist {
| ^^^^^^^^^^^^

error[E0425]: cannot find function `uwu` in module `inner`
--> $DIR/diagnostics-same-crate.rs:45:12
--> $DIR/diagnostics-same-crate.rs:47:12
|
LL | inner::uwu();
| ^^^ not found in `inner`
|
note: found an item that was configured out
--> $DIR/diagnostics-same-crate.rs:3:12
--> $DIR/diagnostics-same-crate.rs:5:12
|
LL | pub fn uwu() {}
| ^^^

error[E0425]: cannot find function `meow` in module `inner::right`
--> $DIR/diagnostics-same-crate.rs:54:19
--> $DIR/diagnostics-same-crate.rs:56:19
|
LL | inner::right::meow();
| ^^^^ not found in `inner::right`
|
note: found an item that was configured out
--> $DIR/diagnostics-same-crate.rs:22:16
--> $DIR/diagnostics-same-crate.rs:24:16
|
LL | pub fn meow() {}
| ^^^^
= note: the item is gated behind the `what-a-cool-feature` feature

error[E0425]: cannot find function `uwu` in this scope
--> $DIR/diagnostics-same-crate.rs:41:5
--> $DIR/diagnostics-same-crate.rs:43:5
|
LL | uwu();
| ^^^ not found in this scope

error[E0425]: cannot find function `vanished` in this scope
--> $DIR/diagnostics-same-crate.rs:61:5
--> $DIR/diagnostics-same-crate.rs:63:5
|
LL | vanished();
| ^^^^^^^^ not found in this scope
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/cfg/expanded-cfg.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//@ check-pass

#![allow(unexpected_cfgs)] // since we different cfgs

macro_rules! mac {
{} => {
#[cfg(attr)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//@ check-fail
//@ compile-flags:--cfg foo
//@ compile-flags:--cfg foo --check-cfg=cfg(foo)

#![cfg_attr(foo, crate_type="bin")]
//~^ERROR `crate_type` within
Expand Down
1 change: 1 addition & 0 deletions tests/ui/check-cfg/allow-at-crate-level.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This test check that #![allow(unexpected_cfgs)] works with --cfg
//
//@ check-pass
//@ no-auto-check-cfg
//@ compile-flags: --cfg=unexpected --check-cfg=cfg()

#![allow(unexpected_cfgs)]
Expand Down
1 change: 1 addition & 0 deletions tests/ui/check-cfg/allow-macro-cfg.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This test check that local #[allow(unexpected_cfgs)] works
//
//@ check-pass
//@ no-auto-check-cfg
//@ compile-flags: --check-cfg=cfg()

#[allow(unexpected_cfgs)]
Expand Down
1 change: 1 addition & 0 deletions tests/ui/check-cfg/allow-same-level.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This test check that #[allow(unexpected_cfgs)] doesn't work if put on the same level
//
//@ check-pass
//@ no-auto-check-cfg
//@ compile-flags: --check-cfg=cfg()

#[allow(unexpected_cfgs)]
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/check-cfg/allow-same-level.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unexpected `cfg` condition name: `FALSE`
--> $DIR/allow-same-level.rs:7:7
--> $DIR/allow-same-level.rs:8:7
|
LL | #[cfg(FALSE)]
| ^^^^^
Expand Down
1 change: 1 addition & 0 deletions tests/ui/check-cfg/allow-top-level.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This test check that a top-level #![allow(unexpected_cfgs)] works
//
//@ check-pass
//@ no-auto-check-cfg
//@ compile-flags: --check-cfg=cfg()

#![allow(unexpected_cfgs)]
Expand Down
1 change: 1 addition & 0 deletions tests/ui/check-cfg/allow-upper-level.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This test check that #[allow(unexpected_cfgs)] work if put on an upper level
//
//@ check-pass
//@ no-auto-check-cfg
//@ compile-flags: --check-cfg=cfg()

#[allow(unexpected_cfgs)]
Expand Down
Loading
Loading