From 5d16c5423c20b3400f3286359a620378e93323d8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 8 Dec 2023 21:02:22 +1100 Subject: [PATCH 01/19] Extend tidy alphabetical checking to `tests/`. This is desired for #118702. --- src/tools/tidy/src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 80e58ba00fc2f..9f92b8995b79e 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -132,6 +132,7 @@ fn main() { check!(edition, &library_path); check!(alphabetical, &src_path); + check!(alphabetical, &tests_path); check!(alphabetical, &compiler_path); check!(alphabetical, &library_path); From a0028143eae1c0a1cdc6991d2b037434031d56e2 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 7 Dec 2023 12:04:00 +0100 Subject: [PATCH 02/19] Strengthen well known check-cfg names and values test --- compiler/rustc_session/src/config.rs | 3 + tests/ui/check-cfg/well-known-values.rs | 107 +++++++-- tests/ui/check-cfg/well-known-values.stderr | 240 +++++++++++++++++--- 3 files changed, 294 insertions(+), 56 deletions(-) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index e694e150b314b..b9e7446d1e5ee 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -1422,6 +1422,9 @@ impl CheckCfg { }; // NOTE: This should be kept in sync with `default_configuration` + // + // When adding a new config here you should also update + // `tests/ui/check-cfg/well-known-values.rs`. let panic_values = &PanicStrategy::all(); diff --git a/tests/ui/check-cfg/well-known-values.rs b/tests/ui/check-cfg/well-known-values.rs index 8b56c8729d844..39a470c202ff1 100644 --- a/tests/ui/check-cfg/well-known-values.rs +++ b/tests/ui/check-cfg/well-known-values.rs @@ -1,41 +1,104 @@ -// This test check that we lint on non well known values and that we don't lint on well known -// values +// This test check that we recognize all the well known config names +// and that we correctly lint on unexpected values. +// +// This test also serve as an "anti-regression" for the well known +// values since the suggestion shows them. // // check-pass // compile-flags: --check-cfg=cfg() -Z unstable-options -#[cfg(target_os = "linuz")] +#![feature(cfg_overflow_checks)] +#![feature(cfg_relocation_model)] +#![feature(cfg_sanitize)] +#![feature(cfg_target_abi)] +#![feature(cfg_target_has_atomic)] +#![feature(cfg_target_has_atomic_equal_alignment)] +#![feature(cfg_target_thread_local)] + +// This part makes sure that none of the well known names are +// unexpected. +// +// BUT to make sure that no expected values changes without +// being noticed we pass them a obviously wrong value so the +// diagnostic prints the list of expected values. +#[cfg(any( + // tidy-alphabetical-start + debug_assertions = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + doc = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + doctest = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + miri = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + overflow_checks = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + panic = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + proc_macro = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + relocation_model = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + sanitize = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_abi = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_arch = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_endian = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_env = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_family = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_feature = "_UNEXPECTED_VALUE", // currently *any* values are "expected" + target_has_atomic = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_has_atomic_equal_alignment = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_has_atomic_load_store = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_os = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_pointer_width = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_thread_local = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + target_vendor = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + test = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + unix = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + windows = "_UNEXPECTED_VALUE", + //~^ WARN unexpected `cfg` condition value + // tidy-alphabetical-end +))] +fn unexpected_values() {} + +#[cfg(target_os = "linuz")] // testing that we suggest `linux` //~^ WARNING unexpected `cfg` condition value fn target_os_linux_misspell() {} +// The #[cfg]s below serve as a safeguard to make sure we +// don't lint when using an expected well-known name and +// value, only a small subset of all possible expected +// configs are tested, since we already test the names +// above and don't need to test all values, just different +// combinations (without value, with value, both...). + #[cfg(target_os = "linux")] fn target_os_linux() {} -#[cfg(target_has_atomic = "0")] -//~^ WARNING unexpected `cfg` condition value -fn target_has_atomic_invalid() {} - #[cfg(target_has_atomic = "8")] -fn target_has_atomic() {} +fn target_has_atomic_8() {} -#[cfg(unix = "aa")] -//~^ WARNING unexpected `cfg` condition value -fn unix_with_value() {} +#[cfg(target_has_atomic)] +fn target_has_atomic() {} #[cfg(unix)] fn unix() {} -#[cfg(miri = "miri")] -//~^ WARNING unexpected `cfg` condition value -fn miri_with_value() {} - -#[cfg(miri)] -fn miri() {} - -#[cfg(doc = "linux")] -//~^ WARNING unexpected `cfg` condition value -fn doc_with_value() {} - #[cfg(doc)] fn doc() {} diff --git a/tests/ui/check-cfg/well-known-values.stderr b/tests/ui/check-cfg/well-known-values.stderr index 6877d8f5bb727..4044bcd2bb0af 100644 --- a/tests/ui/check-cfg/well-known-values.stderr +++ b/tests/ui/check-cfg/well-known-values.stderr @@ -1,53 +1,225 @@ -warning: unexpected `cfg` condition value: `linuz` - --> $DIR/well-known-values.rs:7:7 +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:26:5 | -LL | #[cfg(target_os = "linuz")] - | ^^^^^^^^^^^^------- - | | - | help: there is a expected value with a similar name: `"linux"` +LL | debug_assertions = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^---------------------- + | | + | help: remove the value | - = note: expected values for `target_os` are: `aix`, `android`, `cuda`, `dragonfly`, `emscripten`, `espidf`, `freebsd`, `fuchsia`, `haiku`, `hermit`, `horizon`, `hurd`, `illumos`, `ios`, `l4re`, `linux`, `macos`, `netbsd`, `none`, `nto`, `openbsd`, `psp`, `redox`, `solaris`, `solid_asp3`, `teeos`, `tvos`, `uefi`, `unknown`, `vita`, `vxworks`, `wasi`, `watchos`, `windows`, `xous` + = note: no expected value for `debug_assertions` = note: `#[warn(unexpected_cfgs)]` on by default -warning: unexpected `cfg` condition value: `0` - --> $DIR/well-known-values.rs:14:7 +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:28:5 + | +LL | doc = "_UNEXPECTED_VALUE", + | ^^^---------------------- + | | + | help: remove the value + | + = note: no expected value for `doc` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:30:5 + | +LL | doctest = "_UNEXPECTED_VALUE", + | ^^^^^^^---------------------- + | | + | help: remove the value + | + = note: no expected value for `doctest` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:32:5 + | +LL | miri = "_UNEXPECTED_VALUE", + | ^^^^---------------------- + | | + | help: remove the value + | + = note: no expected value for `miri` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:34:5 + | +LL | overflow_checks = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^---------------------- + | | + | help: remove the value + | + = note: no expected value for `overflow_checks` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:36:5 | -LL | #[cfg(target_has_atomic = "0")] - | ^^^^^^^^^^^^^^^^^^^^--- - | | - | help: there is a expected value with a similar name: `"8"` +LL | panic = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `panic` are: `abort`, `unwind` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:38:5 + | +LL | proc_macro = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^---------------------- + | | + | help: remove the value + | + = note: no expected value for `proc_macro` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:40:5 + | +LL | relocation_model = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `relocation_model` are: `dynamic-no-pic`, `pic`, `pie`, `ropi`, `ropi-rwpi`, `rwpi`, `static` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:42:5 + | +LL | sanitize = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `sanitize` are: `address`, `cfi`, `hwaddress`, `kcfi`, `kernel-address`, `leak`, `memory`, `memtag`, `safestack`, `shadow-call-stack`, `thread` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:44:5 + | +LL | target_abi = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_abi` are: ``, `abi64`, `abiv2`, `abiv2hf`, `eabi`, `eabihf`, `elf`, `fortanix`, `ilp32`, `llvm`, `macabi`, `sim`, `softfloat`, `spe`, `uwp`, `vec-extabi`, `x32` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:46:5 + | +LL | target_arch = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_arch` are: `aarch64`, `arm`, `avr`, `bpf`, `csky`, `hexagon`, `loongarch64`, `m68k`, `mips`, `mips32r6`, `mips64`, `mips64r6`, `msp430`, `nvptx64`, `powerpc`, `powerpc64`, `riscv32`, `riscv64`, `s390x`, `sparc`, `sparc64`, `wasm32`, `wasm64`, `x86`, `x86_64` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:48:5 + | +LL | target_endian = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_endian` are: `big`, `little` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:50:5 + | +LL | target_env = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_env` are: ``, `eabihf`, `gnu`, `gnueabihf`, `msvc`, `musl`, `newlib`, `nto70`, `nto71`, `ohos`, `psx`, `relibc`, `sgx`, `uclibc` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:52:5 + | +LL | target_family = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_family` are: `unix`, `wasm`, `windows` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:55:5 + | +LL | target_has_atomic = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: expected values for `target_has_atomic` are: (none), `128`, `16`, `32`, `64`, `8`, `ptr` -warning: unexpected `cfg` condition value: `aa` - --> $DIR/well-known-values.rs:21:7 +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:57:5 | -LL | #[cfg(unix = "aa")] - | ^^^^------- - | | - | help: remove the value +LL | target_has_atomic_equal_alignment = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_has_atomic_equal_alignment` are: (none), `128`, `16`, `32`, `64`, `8`, `ptr` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:59:5 + | +LL | target_has_atomic_load_store = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_has_atomic_load_store` are: (none), `128`, `16`, `32`, `64`, `8`, `ptr` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:61:5 + | +LL | target_os = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_os` are: `aix`, `android`, `cuda`, `dragonfly`, `emscripten`, `espidf`, `freebsd`, `fuchsia`, `haiku`, `hermit`, `horizon`, `hurd`, `illumos`, `ios`, `l4re`, `linux`, `macos`, `netbsd`, `none`, `nto`, `openbsd`, `psp`, `redox`, `solaris`, `solid_asp3`, `teeos`, `tvos`, `uefi`, `unknown`, `vita`, `vxworks`, `wasi`, `watchos`, `windows`, `xous` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:63:5 + | +LL | target_pointer_width = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_pointer_width` are: `16`, `32`, `64` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:65:5 + | +LL | target_thread_local = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^---------------------- + | | + | help: remove the value + | + = note: no expected value for `target_thread_local` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:67:5 + | +LL | target_vendor = "_UNEXPECTED_VALUE", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_vendor` are: `apple`, `espressif`, `fortanix`, `ibm`, `kmc`, `nintendo`, `nvidia`, `pc`, `sony`, `sun`, `unikraft`, `unknown`, `uwp`, `wrs` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:69:5 + | +LL | test = "_UNEXPECTED_VALUE", + | ^^^^---------------------- + | | + | help: remove the value + | + = note: no expected value for `test` + +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:71:5 + | +LL | unix = "_UNEXPECTED_VALUE", + | ^^^^---------------------- + | | + | help: remove the value | = note: no expected value for `unix` -warning: unexpected `cfg` condition value: `miri` - --> $DIR/well-known-values.rs:28:7 +warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` + --> $DIR/well-known-values.rs:73:5 | -LL | #[cfg(miri = "miri")] - | ^^^^--------- - | | - | help: remove the value +LL | windows = "_UNEXPECTED_VALUE", + | ^^^^^^^---------------------- + | | + | help: remove the value | - = note: no expected value for `miri` + = note: no expected value for `windows` -warning: unexpected `cfg` condition value: `linux` - --> $DIR/well-known-values.rs:35:7 +warning: unexpected `cfg` condition value: `linuz` + --> $DIR/well-known-values.rs:79:7 | -LL | #[cfg(doc = "linux")] - | ^^^---------- - | | - | help: remove the value +LL | #[cfg(target_os = "linuz")] // testing that we suggest `linux` + | ^^^^^^^^^^^^------- + | | + | help: there is a expected value with a similar name: `"linux"` | - = note: no expected value for `doc` + = note: expected values for `target_os` are: `aix`, `android`, `cuda`, `dragonfly`, `emscripten`, `espidf`, `freebsd`, `fuchsia`, `haiku`, `hermit`, `horizon`, `hurd`, `illumos`, `ios`, `l4re`, `linux`, `macos`, `netbsd`, `none`, `nto`, `openbsd`, `psp`, `redox`, `solaris`, `solid_asp3`, `teeos`, `tvos`, `uefi`, `unknown`, `vita`, `vxworks`, `wasi`, `watchos`, `windows`, `xous` -warning: 5 warnings emitted +warning: 25 warnings emitted From 5a17ee75b363350dad829c7a5f93a320a3393774 Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 8 Dec 2023 00:42:57 +0100 Subject: [PATCH 03/19] Avoid target_os and target_arch in some check-cfg tests as they unnecessarily clutter the diagnostic output and make the experience of adding a new target to the compiler more painful than it should be. target_os and target_arch are still being tested in the well-known-values.rs test, but in one place. --- tests/ui/check-cfg/compact-values.rs | 2 +- tests/ui/check-cfg/compact-values.stderr | 6 +++--- tests/ui/check-cfg/values-target-json.rs | 4 ---- tests/ui/check-cfg/values-target-json.stderr | 13 ------------- 4 files changed, 4 insertions(+), 21 deletions(-) delete mode 100644 tests/ui/check-cfg/values-target-json.stderr diff --git a/tests/ui/check-cfg/compact-values.rs b/tests/ui/check-cfg/compact-values.rs index 13c072fe9206d..80cf75d2770d5 100644 --- a/tests/ui/check-cfg/compact-values.rs +++ b/tests/ui/check-cfg/compact-values.rs @@ -8,7 +8,7 @@ #[cfg(target(os = "linux", arch = "arm"))] pub fn expected() {} -#[cfg(target(os = "linux", arch = "X"))] +#[cfg(target(os = "linux", pointer_width = "X"))] //~^ WARNING unexpected `cfg` condition value pub fn unexpected() {} diff --git a/tests/ui/check-cfg/compact-values.stderr b/tests/ui/check-cfg/compact-values.stderr index bb2f4915b5ef6..819b789c3e53b 100644 --- a/tests/ui/check-cfg/compact-values.stderr +++ b/tests/ui/check-cfg/compact-values.stderr @@ -1,10 +1,10 @@ warning: unexpected `cfg` condition value: `X` --> $DIR/compact-values.rs:11:28 | -LL | #[cfg(target(os = "linux", arch = "X"))] - | ^^^^^^^^^^ +LL | #[cfg(target(os = "linux", pointer_width = "X"))] + | ^^^^^^^^^^^^^^^^^^^ | - = note: expected values for `target_arch` are: `aarch64`, `arm`, `avr`, `bpf`, `csky`, `hexagon`, `loongarch64`, `m68k`, `mips`, `mips32r6`, `mips64`, `mips64r6`, `msp430`, `nvptx64`, `powerpc`, `powerpc64`, `riscv32`, `riscv64`, `s390x`, `sparc`, `sparc64`, `wasm32`, `wasm64`, `x86`, `x86_64` + = note: expected values for `target_pointer_width` are: `16`, `32`, `64` = note: `#[warn(unexpected_cfgs)]` on by default warning: 1 warning emitted diff --git a/tests/ui/check-cfg/values-target-json.rs b/tests/ui/check-cfg/values-target-json.rs index e4c1b54ccccfd..47ac79e0dbffd 100644 --- a/tests/ui/check-cfg/values-target-json.rs +++ b/tests/ui/check-cfg/values-target-json.rs @@ -10,10 +10,6 @@ #[lang = "sized"] trait Sized {} -#[cfg(target_os = "linuz")] -//~^ WARNING unexpected `cfg` condition value -fn target_os_linux_misspell() {} - #[cfg(target_os = "linux")] fn target_os_linux() {} diff --git a/tests/ui/check-cfg/values-target-json.stderr b/tests/ui/check-cfg/values-target-json.stderr deleted file mode 100644 index e71149f337f58..0000000000000 --- a/tests/ui/check-cfg/values-target-json.stderr +++ /dev/null @@ -1,13 +0,0 @@ -warning: unexpected `cfg` condition value: `linuz` - --> $DIR/values-target-json.rs:13:7 - | -LL | #[cfg(target_os = "linuz")] - | ^^^^^^^^^^^^------- - | | - | help: there is a expected value with a similar name: `"linux"` - | - = note: expected values for `target_os` are: `aix`, `android`, `cuda`, `dragonfly`, `emscripten`, `ericos`, `espidf`, `freebsd`, `fuchsia`, `haiku`, `hermit`, `horizon`, `hurd`, `illumos`, `ios`, `l4re`, `linux`, `macos`, `netbsd`, `none`, `nto`, `openbsd`, `psp`, `redox`, `solaris`, `solid_asp3`, `teeos`, `tvos`, `uefi`, `unknown`, `vita`, `vxworks`, `wasi`, `watchos`, `windows`, `xous` - = note: `#[warn(unexpected_cfgs)]` on by default - -warning: 1 warning emitted - From d2d742c4ccc8c18fe13bfc42b3a7b9b66486b294 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 23 Nov 2023 13:52:50 +1100 Subject: [PATCH 04/19] coverage: Add a dedicated test for coverage of `if !` --- tests/coverage/if_not.cov-map | 39 ++++++++++++++++++++++++++++++++++ tests/coverage/if_not.coverage | 38 +++++++++++++++++++++++++++++++++ tests/coverage/if_not.rs | 37 ++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+) create mode 100644 tests/coverage/if_not.cov-map create mode 100644 tests/coverage/if_not.coverage create mode 100644 tests/coverage/if_not.rs diff --git a/tests/coverage/if_not.cov-map b/tests/coverage/if_not.cov-map new file mode 100644 index 0000000000000..73627308516d2 --- /dev/null +++ b/tests/coverage/if_not.cov-map @@ -0,0 +1,39 @@ +Function name: if_not::if_not +Raw bytes (86): 0x[01, 01, 10, 01, 05, 05, 02, 3f, 09, 05, 02, 09, 3a, 3f, 09, 05, 02, 37, 0d, 09, 3a, 3f, 09, 05, 02, 0d, 32, 37, 0d, 09, 3a, 3f, 09, 05, 02, 0a, 01, 04, 01, 03, 0d, 02, 04, 05, 02, 06, 05, 02, 06, 00, 07, 3f, 04, 09, 00, 0d, 3a, 01, 05, 02, 06, 09, 02, 06, 00, 07, 37, 04, 09, 00, 0d, 32, 01, 05, 02, 06, 0d, 02, 0c, 02, 06, 2f, 03, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 16 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) +- expression 2 operands: lhs = Expression(15, Add), rhs = Counter(2) +- expression 3 operands: lhs = Counter(1), rhs = Expression(0, Sub) +- expression 4 operands: lhs = Counter(2), rhs = Expression(14, Sub) +- expression 5 operands: lhs = Expression(15, Add), rhs = Counter(2) +- expression 6 operands: lhs = Counter(1), rhs = Expression(0, Sub) +- expression 7 operands: lhs = Expression(13, Add), rhs = Counter(3) +- expression 8 operands: lhs = Counter(2), rhs = Expression(14, Sub) +- expression 9 operands: lhs = Expression(15, Add), rhs = Counter(2) +- expression 10 operands: lhs = Counter(1), rhs = Expression(0, Sub) +- expression 11 operands: lhs = Counter(3), rhs = Expression(12, Sub) +- expression 12 operands: lhs = Expression(13, Add), rhs = Counter(3) +- expression 13 operands: lhs = Counter(2), rhs = Expression(14, Sub) +- expression 14 operands: lhs = Expression(15, Add), rhs = Counter(2) +- expression 15 operands: lhs = Counter(1), rhs = Expression(0, Sub) +Number of file 0 mappings: 10 +- Code(Counter(0)) at (prev + 4, 1) to (start + 3, 13) +- Code(Expression(0, Sub)) at (prev + 4, 5) to (start + 2, 6) + = (c0 - c1) +- Code(Counter(1)) at (prev + 2, 6) to (start + 0, 7) +- Code(Expression(15, Add)) at (prev + 4, 9) to (start + 0, 13) + = (c1 + (c0 - c1)) +- Code(Expression(14, Sub)) at (prev + 1, 5) to (start + 2, 6) + = ((c1 + (c0 - c1)) - c2) +- Code(Counter(2)) at (prev + 2, 6) to (start + 0, 7) +- Code(Expression(13, Add)) at (prev + 4, 9) to (start + 0, 13) + = (c2 + ((c1 + (c0 - c1)) - c2)) +- Code(Expression(12, Sub)) at (prev + 1, 5) to (start + 2, 6) + = ((c2 + ((c1 + (c0 - c1)) - c2)) - c3) +- Code(Counter(3)) at (prev + 2, 12) to (start + 2, 6) +- Code(Expression(11, Add)) at (prev + 3, 1) to (start + 0, 2) + = (c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) + diff --git a/tests/coverage/if_not.coverage b/tests/coverage/if_not.coverage new file mode 100644 index 0000000000000..4c8ef9a84e22a --- /dev/null +++ b/tests/coverage/if_not.coverage @@ -0,0 +1,38 @@ + LL| |#![feature(coverage_attribute)] + LL| |// edition: 2021 + LL| | + LL| 12|fn if_not(cond: bool) { + LL| 12| if + LL| 12| ! + LL| 12| cond + LL| 4| { + LL| 4| println!("cond was false"); + LL| 8| } + LL| | + LL| | if + LL| | ! + LL| 12| cond + LL| 4| { + LL| 4| println!("cond was false"); + LL| 8| } + LL| | + LL| | if + LL| | ! + LL| 12| cond + LL| 4| { + LL| 4| println!("cond was false"); + LL| 8| } else { + LL| 8| println!("cond was true"); + LL| 8| } + LL| 12|} + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | for _ in 0..8 { + LL| | if_not(std::hint::black_box(true)); + LL| | } + LL| | for _ in 0..4 { + LL| | if_not(std::hint::black_box(false)); + LL| | } + LL| |} + diff --git a/tests/coverage/if_not.rs b/tests/coverage/if_not.rs new file mode 100644 index 0000000000000..4f45ae0b3d447 --- /dev/null +++ b/tests/coverage/if_not.rs @@ -0,0 +1,37 @@ +#![feature(coverage_attribute)] +// edition: 2021 + +fn if_not(cond: bool) { + if + ! + cond + { + println!("cond was false"); + } + + if + ! + cond + { + println!("cond was false"); + } + + if + ! + cond + { + println!("cond was false"); + } else { + println!("cond was true"); + } +} + +#[coverage(off)] +fn main() { + for _ in 0..8 { + if_not(std::hint::black_box(true)); + } + for _ in 0..4 { + if_not(std::hint::black_box(false)); + } +} From 44b47aa976d6a6bdd7eb1f99a8ee5270afbe993e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 23 Nov 2023 11:50:39 +1100 Subject: [PATCH 05/19] coverage: Add `CoverageKind::SpanMarker` for including extra spans in MIR There are cases where coverage instrumentation wants to show a span for some syntax element, but there is no MIR node that naturally carries that span, so the instrumentor can't see it. MIR building can now use this new kind of coverage statement to deliberately include those spans in MIR, attached to a dummy statement that has no other effect. --- .../rustc_codegen_llvm/src/coverageinfo/mod.rs | 3 +++ compiler/rustc_middle/src/mir/coverage.rs | 8 ++++++++ compiler/rustc_mir_build/src/build/cfg.rs | 13 +++++++++++++ .../src/coverage/spans/from_mir.rs | 15 ++++++++++++--- 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 7d69756181a94..8386f067bafba 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -100,6 +100,9 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { let Coverage { kind } = coverage; match *kind { + // Span markers are only meaningful during MIR instrumentation, + // and have no effect during codegen. + CoverageKind::SpanMarker => {} CoverageKind::CounterIncrement { id } => { func_coverage.mark_counter_id_seen(id); // We need to explicitly drop the `RefMut` before calling into `instrprof_increment`, diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index f15ee0082cede..ec5edceb26997 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -76,6 +76,13 @@ impl Debug for CovTerm { #[derive(Clone, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub enum CoverageKind { + /// Marks a span that might otherwise not be represented in MIR, so that + /// coverage instrumentation can associate it with its enclosing block/BCB. + /// + /// Only used by the `InstrumentCoverage` pass, and has no effect during + /// codegen. + SpanMarker, + /// Marks the point in MIR control flow represented by a coverage counter. /// /// This is eventually lowered to `llvm.instrprof.increment` in LLVM IR. @@ -99,6 +106,7 @@ impl Debug for CoverageKind { fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { use CoverageKind::*; match self { + SpanMarker => write!(fmt, "SpanMarker"), CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()), ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()), } diff --git a/compiler/rustc_mir_build/src/build/cfg.rs b/compiler/rustc_mir_build/src/build/cfg.rs index fddcf9de7c7c9..2bd0e28973101 100644 --- a/compiler/rustc_mir_build/src/build/cfg.rs +++ b/compiler/rustc_mir_build/src/build/cfg.rs @@ -101,6 +101,19 @@ impl<'tcx> CFG<'tcx> { self.push(block, stmt); } + /// Adds a dummy statement whose only role is to associate a span with its + /// enclosing block for the purposes of coverage instrumentation. + /// + /// This results in more accurate coverage reports for certain kinds of + /// syntax (e.g. `continue` or `if !`) that would otherwise not appear in MIR. + pub(crate) fn push_coverage_span_marker(&mut self, block: BasicBlock, source_info: SourceInfo) { + let kind = StatementKind::Coverage(Box::new(Coverage { + kind: coverage::CoverageKind::SpanMarker, + })); + let stmt = Statement { source_info, kind }; + self.push(block, stmt); + } + pub(crate) fn terminate( &mut self, block: BasicBlock, diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index e1531f2c239bf..6f7d8d9dd7551 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -92,13 +92,13 @@ fn is_closure(statement: &Statement<'_>) -> bool { /// If the MIR `Statement` has a span contributive to computing coverage spans, /// return it; otherwise return `None`. fn filtered_statement_span(statement: &Statement<'_>) -> Option { + use mir::coverage::CoverageKind; + match statement.kind { // These statements have spans that are often outside the scope of the executed source code // for their parent `BasicBlock`. StatementKind::StorageLive(_) | StatementKind::StorageDead(_) - // Coverage should not be encountered, but don't inject coverage coverage - | StatementKind::Coverage(_) // Ignore `ConstEvalCounter`s | StatementKind::ConstEvalCounter // Ignore `Nop`s @@ -122,9 +122,13 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option { // If and when the Issue is resolved, remove this special case match pattern: StatementKind::FakeRead(box (FakeReadCause::ForGuardBinding, _)) => None, - // Retain spans from all other statements + // Retain spans from most other statements. StatementKind::FakeRead(box (_, _)) // Not including `ForGuardBinding` | StatementKind::Intrinsic(..) + | StatementKind::Coverage(box mir::Coverage { + // The purpose of `SpanMarker` is to be matched and accepted here. + kind: CoverageKind::SpanMarker + }) | StatementKind::Assign(_) | StatementKind::SetDiscriminant { .. } | StatementKind::Deinit(..) @@ -133,6 +137,11 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option { | StatementKind::AscribeUserType(_, _) => { Some(statement.source_info.span) } + + StatementKind::Coverage(box mir::Coverage { + // These coverage statements should not exist prior to coverage instrumentation. + kind: CoverageKind::CounterIncrement { .. } | CoverageKind::ExpressionUsed { .. } + }) => bug!("Unexpected coverage statement found during coverage instrumentation: {statement:?}"), } } From 98166358a99aea4ded1a42a3c113aac764c752ed Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 23 Nov 2023 11:59:13 +1100 Subject: [PATCH 06/19] coverage: Use `SpanMarker` to mark `continue` expressions. This replaces the previous workaround, which was to inject a dummy `Assign` statement. --- compiler/rustc_mir_build/src/build/scope.rs | 24 +++++++-------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 993fee95895ce..88fcaa0a41cc0 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -90,7 +90,6 @@ use rustc_index::{IndexSlice, IndexVec}; use rustc_middle::middle::region; use rustc_middle::mir::*; use rustc_middle::thir::{Expr, LintLevel}; -use rustc_middle::ty::Ty; use rustc_session::lint::Level; use rustc_span::{Span, DUMMY_SP}; @@ -660,14 +659,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { (None, Some(_)) => { panic!("`return`, `become` and `break` with value and must have a destination") } - (None, None) if self.tcx.sess.instrument_coverage() => { - // Unlike `break` and `return`, which push an `Assign` statement to MIR, from which - // a Coverage code region can be generated, `continue` needs no `Assign`; but - // without one, the `InstrumentCoverage` MIR pass cannot generate a code region for - // `continue`. Coverage will be missing unless we add a dummy `Assign` to MIR. - self.add_dummy_assignment(span, block, source_info); + (None, None) => { + if self.tcx.sess.instrument_coverage() { + // Normally we wouldn't build any MIR in this case, but that makes it + // harder for coverage instrumentation to extract a relevant span for + // `continue` expressions. So here we inject a dummy statement with the + // desired span. + self.cfg.push_coverage_span_marker(block, source_info); + } } - (None, None) => {} } let region_scope = self.scopes.breakable_scopes[break_index].region_scope; @@ -723,14 +723,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.cfg.terminate(block, source_info, TerminatorKind::UnwindResume); } - // Add a dummy `Assign` statement to the CFG, with the span for the source code's `continue` - // statement. - fn add_dummy_assignment(&mut self, span: Span, block: BasicBlock, source_info: SourceInfo) { - let local_decl = LocalDecl::new(Ty::new_unit(self.tcx), span); - let temp_place = Place::from(self.local_decls.push(local_decl)); - self.cfg.push_assign_unit(block, source_info, temp_place, self.tcx); - } - fn leave_top_scope(&mut self, block: BasicBlock) -> BasicBlock { // If we are emitting a `drop` statement, we need to have the cached // diverge cleanup pads ready in case that drop panics. From d90fd027c887247caa501db5f49352fdfab769d9 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 23 Nov 2023 12:50:40 +1100 Subject: [PATCH 07/19] coverage: Use `SpanMarker` to mark the full condition of `if !` When MIR is built for an if-not expression, the `!` part of the condition doesn't correspond to any MIR statement, so coverage instrumentation normally can't see it. We can fix that by deliberately injecting a dummy statement whose sole purpose is to associate that span with its enclosing block. --- compiler/rustc_mir_build/src/build/matches/mod.rs | 5 +++++ tests/coverage/if_not.cov-map | 10 +++++----- tests/coverage/if_not.coverage | 4 ++-- tests/coverage/lazy_boolean.cov-map | 8 ++++---- tests/coverage/lazy_boolean.coverage | 2 +- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 90f950d59d551..541b87af7977b 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -90,6 +90,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let local_scope = this.local_scope(); let (success_block, failure_block) = this.in_if_then_scope(local_scope, expr_span, |this| { + // Help out coverage instrumentation by injecting a dummy statement with + // the original condition's span (including `!`). This fixes #115468. + if this.tcx.sess.instrument_coverage() { + this.cfg.push_coverage_span_marker(block, this.source_info(expr_span)); + } this.then_else_break( block, &this.thir[arg], diff --git a/tests/coverage/if_not.cov-map b/tests/coverage/if_not.cov-map index 73627308516d2..fb893e3796061 100644 --- a/tests/coverage/if_not.cov-map +++ b/tests/coverage/if_not.cov-map @@ -1,5 +1,5 @@ Function name: if_not::if_not -Raw bytes (86): 0x[01, 01, 10, 01, 05, 05, 02, 3f, 09, 05, 02, 09, 3a, 3f, 09, 05, 02, 37, 0d, 09, 3a, 3f, 09, 05, 02, 0d, 32, 37, 0d, 09, 3a, 3f, 09, 05, 02, 0a, 01, 04, 01, 03, 0d, 02, 04, 05, 02, 06, 05, 02, 06, 00, 07, 3f, 04, 09, 00, 0d, 3a, 01, 05, 02, 06, 09, 02, 06, 00, 07, 37, 04, 09, 00, 0d, 32, 01, 05, 02, 06, 0d, 02, 0c, 02, 06, 2f, 03, 01, 00, 02] +Raw bytes (86): 0x[01, 01, 10, 01, 05, 05, 02, 3f, 09, 05, 02, 09, 3a, 3f, 09, 05, 02, 37, 0d, 09, 3a, 3f, 09, 05, 02, 0d, 32, 37, 0d, 09, 3a, 3f, 09, 05, 02, 0a, 01, 04, 01, 03, 0d, 02, 04, 05, 02, 06, 05, 02, 06, 00, 07, 3f, 03, 09, 01, 0d, 3a, 02, 05, 02, 06, 09, 02, 06, 00, 07, 37, 03, 09, 01, 0d, 32, 02, 05, 02, 06, 0d, 02, 0c, 02, 06, 2f, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 16 @@ -24,14 +24,14 @@ Number of file 0 mappings: 10 - Code(Expression(0, Sub)) at (prev + 4, 5) to (start + 2, 6) = (c0 - c1) - Code(Counter(1)) at (prev + 2, 6) to (start + 0, 7) -- Code(Expression(15, Add)) at (prev + 4, 9) to (start + 0, 13) +- Code(Expression(15, Add)) at (prev + 3, 9) to (start + 1, 13) = (c1 + (c0 - c1)) -- Code(Expression(14, Sub)) at (prev + 1, 5) to (start + 2, 6) +- Code(Expression(14, Sub)) at (prev + 2, 5) to (start + 2, 6) = ((c1 + (c0 - c1)) - c2) - Code(Counter(2)) at (prev + 2, 6) to (start + 0, 7) -- Code(Expression(13, Add)) at (prev + 4, 9) to (start + 0, 13) +- Code(Expression(13, Add)) at (prev + 3, 9) to (start + 1, 13) = (c2 + ((c1 + (c0 - c1)) - c2)) -- Code(Expression(12, Sub)) at (prev + 1, 5) to (start + 2, 6) +- Code(Expression(12, Sub)) at (prev + 2, 5) to (start + 2, 6) = ((c2 + ((c1 + (c0 - c1)) - c2)) - c3) - Code(Counter(3)) at (prev + 2, 12) to (start + 2, 6) - Code(Expression(11, Add)) at (prev + 3, 1) to (start + 0, 2) diff --git a/tests/coverage/if_not.coverage b/tests/coverage/if_not.coverage index 4c8ef9a84e22a..41838b8513f6a 100644 --- a/tests/coverage/if_not.coverage +++ b/tests/coverage/if_not.coverage @@ -10,14 +10,14 @@ LL| 8| } LL| | LL| | if - LL| | ! + LL| 12| ! LL| 12| cond LL| 4| { LL| 4| println!("cond was false"); LL| 8| } LL| | LL| | if - LL| | ! + LL| 12| ! LL| 12| cond LL| 4| { LL| 4| println!("cond was false"); diff --git a/tests/coverage/lazy_boolean.cov-map b/tests/coverage/lazy_boolean.cov-map index 0ad393c40fa77..2d1ff24e62d56 100644 --- a/tests/coverage/lazy_boolean.cov-map +++ b/tests/coverage/lazy_boolean.cov-map @@ -1,5 +1,5 @@ Function name: lazy_boolean::main -Raw bytes (636): 0x[01, 01, a4, 01, 01, 05, 09, 8a, 05, 8f, 05, 09, 05, 02, 05, 02, 8f, 05, 09, 05, 02, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 09, 8a, 05, 8f, 05, 09, 05, 02, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 1d, e2, 04, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, df, 04, 21, 1d, e2, 04, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 21, da, 04, df, 04, 21, 1d, e2, 04, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, d7, 04, 25, 21, da, 04, df, 04, 21, 1d, e2, 04, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 25, d2, 04, d7, 04, 25, 21, da, 04, df, 04, 21, 1d, e2, 04, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 1c, 01, 03, 01, 07, 0f, 05, 07, 10, 04, 06, 02, 04, 06, 00, 07, 87, 05, 02, 09, 00, 11, 8f, 05, 02, 0d, 00, 12, 8a, 05, 02, 0d, 00, 12, ff, 04, 03, 09, 00, 11, 87, 05, 02, 0d, 00, 12, 82, 05, 02, 0d, 00, 12, f7, 04, 02, 09, 00, 11, ff, 04, 00, 14, 00, 19, 11, 00, 1d, 00, 22, ef, 04, 01, 09, 00, 11, f7, 04, 00, 14, 00, 19, 15, 00, 1d, 00, 22, ef, 04, 04, 09, 00, 10, ea, 04, 01, 05, 03, 06, 19, 03, 06, 00, 07, e7, 04, 03, 09, 00, 10, 1d, 01, 05, 03, 06, e2, 04, 05, 05, 03, 06, df, 04, 05, 09, 00, 10, da, 04, 00, 11, 02, 06, 21, 02, 06, 00, 07, d7, 04, 02, 08, 00, 0f, 25, 00, 10, 02, 06, d2, 04, 02, 0c, 02, 06, cf, 04, 03, 01, 00, 02] +Raw bytes (636): 0x[01, 01, a4, 01, 01, 05, 09, 8a, 05, 8f, 05, 09, 05, 02, 05, 02, 8f, 05, 09, 05, 02, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 09, 8a, 05, 8f, 05, 09, 05, 02, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 1d, e2, 04, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, df, 04, 21, 1d, e2, 04, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 21, da, 04, df, 04, 21, 1d, e2, 04, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, d7, 04, 25, 21, da, 04, df, 04, 21, 1d, e2, 04, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 25, d2, 04, d7, 04, 25, 21, da, 04, df, 04, 21, 1d, e2, 04, e7, 04, 1d, 19, ea, 04, ef, 04, 19, 15, f2, 04, f7, 04, 15, 11, fa, 04, ff, 04, 11, 0d, 82, 05, 87, 05, 0d, 09, 8a, 05, 8f, 05, 09, 05, 02, 1c, 01, 03, 01, 07, 0f, 05, 07, 10, 04, 06, 02, 04, 06, 00, 07, 87, 05, 02, 09, 00, 11, 8f, 05, 02, 0d, 00, 12, 8a, 05, 02, 0d, 00, 12, ff, 04, 03, 09, 00, 11, 87, 05, 02, 0d, 00, 12, 82, 05, 02, 0d, 00, 12, f7, 04, 02, 09, 00, 11, ff, 04, 00, 14, 00, 19, 11, 00, 1d, 00, 22, ef, 04, 01, 09, 00, 11, f7, 04, 00, 14, 00, 19, 15, 00, 1d, 00, 22, ef, 04, 03, 09, 01, 10, ea, 04, 02, 05, 03, 06, 19, 03, 06, 00, 07, e7, 04, 03, 09, 00, 10, 1d, 01, 05, 03, 06, e2, 04, 05, 05, 03, 06, df, 04, 05, 08, 00, 10, da, 04, 00, 11, 02, 06, 21, 02, 06, 00, 07, d7, 04, 02, 08, 00, 0f, 25, 00, 10, 02, 06, d2, 04, 02, 0c, 02, 06, cf, 04, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 164 @@ -194,9 +194,9 @@ Number of file 0 mappings: 28 - Code(Expression(157, Add)) at (prev + 0, 20) to (start + 0, 25) = (c4 + ((c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) - c4)) - Code(Counter(5)) at (prev + 0, 29) to (start + 0, 34) -- Code(Expression(155, Add)) at (prev + 4, 9) to (start + 0, 16) +- Code(Expression(155, Add)) at (prev + 3, 9) to (start + 1, 16) = (c5 + ((c4 + ((c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) - c4)) - c5)) -- Code(Expression(154, Sub)) at (prev + 1, 5) to (start + 3, 6) +- Code(Expression(154, Sub)) at (prev + 2, 5) to (start + 3, 6) = ((c5 + ((c4 + ((c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) - c4)) - c5)) - c6) - Code(Counter(6)) at (prev + 3, 6) to (start + 0, 7) - Code(Expression(153, Add)) at (prev + 3, 9) to (start + 0, 16) @@ -204,7 +204,7 @@ Number of file 0 mappings: 28 - Code(Counter(7)) at (prev + 1, 5) to (start + 3, 6) - Code(Expression(152, Sub)) at (prev + 5, 5) to (start + 3, 6) = ((c6 + ((c5 + ((c4 + ((c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) - c4)) - c5)) - c6)) - c7) -- Code(Expression(151, Add)) at (prev + 5, 9) to (start + 0, 16) +- Code(Expression(151, Add)) at (prev + 5, 8) to (start + 0, 16) = (c7 + ((c6 + ((c5 + ((c4 + ((c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) - c4)) - c5)) - c6)) - c7)) - Code(Expression(150, Sub)) at (prev + 0, 17) to (start + 2, 6) = ((c7 + ((c6 + ((c5 + ((c4 + ((c3 + ((c2 + ((c1 + (c0 - c1)) - c2)) - c3)) - c4)) - c5)) - c6)) - c7)) - c8) diff --git a/tests/coverage/lazy_boolean.coverage b/tests/coverage/lazy_boolean.coverage index 8f14082ef6825..2d927a083560f 100644 --- a/tests/coverage/lazy_boolean.coverage +++ b/tests/coverage/lazy_boolean.coverage @@ -32,7 +32,7 @@ ^0 LL| | LL| | if - LL| | ! + LL| 1| ! LL| 1| is_true LL| 0| { LL| 0| a = 2 From e0cd8057c85260e827e417cfcf3c6c861d2c8426 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 6 Dec 2023 18:40:06 +1100 Subject: [PATCH 08/19] coverage: Simplify the heuristic for ignoring `async fn` return spans --- .../rustc_mir_transform/src/coverage/spans.rs | 21 ++++--------------- .../src/coverage/spans/from_mir.rs | 10 +++++++++ 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index c415a8329942a..0124bb7337cba 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -319,29 +319,16 @@ impl<'a> CoverageSpansGenerator<'a> { } } - let prev = self.take_prev(); - debug!(" AT END, adding last prev={prev:?}"); - // Drain any remaining dups into the output. for dup in self.pending_dups.drain(..) { debug!(" ...adding at least one pending dup={:?}", dup); self.refined_spans.push(dup); } - // Async functions wrap a closure that implements the body to be executed. The enclosing - // function is called and returns an `impl Future` without initially executing any of the - // body. To avoid showing the return from the enclosing function as a "covered" return from - // the closure, the enclosing function's `TerminatorKind::Return`s `CoverageSpan` is - // excluded. The closure's `Return` is the only one that will be counted. This provides - // adequate coverage, and more intuitive counts. (Avoids double-counting the closing brace - // of the function body.) - let body_ends_with_closure = if let Some(last_covspan) = self.refined_spans.last() { - last_covspan.is_closure && last_covspan.span.hi() == self.body_span.hi() - } else { - false - }; - - if !body_ends_with_closure { + // There is usually a final span remaining in `prev` after the loop ends, + // so add it to the output as well. + if let Some(prev) = self.some_prev.take() { + debug!(" AT END, adding last prev={prev:?}"); self.refined_spans.push(prev); } diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index e1531f2c239bf..b850b3374bac8 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -44,6 +44,16 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( .then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse()) }); + // The desugaring of an async function includes a closure containing the + // original function body, and a terminator that returns the `impl Future`. + // That terminator will cause a confusing coverage count for the function's + // closing brace, so discard everything after the body closure span. + if let Some(body_closure_index) = + initial_spans.iter().rposition(|covspan| covspan.is_closure && covspan.span == body_span) + { + initial_spans.truncate(body_closure_index + 1); + } + initial_spans } From cec814202a7941f92ed0f3e54d66d95b1ebcd74e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 6 Dec 2023 21:07:43 +1100 Subject: [PATCH 09/19] coverage: Add `#[track_caller]` to the span generator's unwrap methods This should make it easier to investigate unwrap failures in bug reports. --- .../rustc_mir_transform/src/coverage/spans.rs | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 0124bb7337cba..05ad14f1525cd 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -385,38 +385,36 @@ impl<'a> CoverageSpansGenerator<'a> { self.refined_spans.push(macro_name_cov); } + #[track_caller] fn curr(&self) -> &CoverageSpan { - self.some_curr - .as_ref() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + self.some_curr.as_ref().unwrap_or_else(|| bug!("some_curr is None (curr)")) } + #[track_caller] fn curr_mut(&mut self) -> &mut CoverageSpan { - self.some_curr - .as_mut() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + self.some_curr.as_mut().unwrap_or_else(|| bug!("some_curr is None (curr_mut)")) } /// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the /// `curr` coverage span. + #[track_caller] fn take_curr(&mut self) -> CoverageSpan { - self.some_curr.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + self.some_curr.take().unwrap_or_else(|| bug!("some_curr is None (take_curr)")) } + #[track_caller] fn prev(&self) -> &CoverageSpan { - self.some_prev - .as_ref() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + self.some_prev.as_ref().unwrap_or_else(|| bug!("some_prev is None (prev)")) } + #[track_caller] fn prev_mut(&mut self) -> &mut CoverageSpan { - self.some_prev - .as_mut() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + self.some_prev.as_mut().unwrap_or_else(|| bug!("some_prev is None (prev_mut)")) } + #[track_caller] fn take_prev(&mut self) -> CoverageSpan { - self.some_prev.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + self.some_prev.take().unwrap_or_else(|| bug!("some_prev is None (take_prev)")) } /// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the From e01338aeb8d83f373ff2d563b147456a68c751e5 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 8 Dec 2023 15:51:06 +1100 Subject: [PATCH 10/19] coverage: Regression test for unwrapping `prev` when there are no spans --- tests/coverage/no_spans.cov-map | 8 ++++++++ tests/coverage/no_spans.coverage | 30 ++++++++++++++++++++++++++++++ tests/coverage/no_spans.rs | 29 +++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100644 tests/coverage/no_spans.cov-map create mode 100644 tests/coverage/no_spans.coverage create mode 100644 tests/coverage/no_spans.rs diff --git a/tests/coverage/no_spans.cov-map b/tests/coverage/no_spans.cov-map new file mode 100644 index 0000000000000..9915fc52e6db6 --- /dev/null +++ b/tests/coverage/no_spans.cov-map @@ -0,0 +1,8 @@ +Function name: no_spans::affected_function::{closure#0} +Raw bytes (9): 0x[01, 01, 00, 01, 01, 1b, 0c, 00, 0e] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 27, 12) to (start + 0, 14) + diff --git a/tests/coverage/no_spans.coverage b/tests/coverage/no_spans.coverage new file mode 100644 index 0000000000000..e55177698a261 --- /dev/null +++ b/tests/coverage/no_spans.coverage @@ -0,0 +1,30 @@ + LL| |#![feature(coverage_attribute)] + LL| |// edition: 2021 + LL| | + LL| |// If the span extractor can't find any relevant spans for a function, the + LL| |// refinement loop will terminate with nothing in its `prev` slot. If the + LL| |// subsequent code tries to unwrap `prev`, it will panic. + LL| |// + LL| |// This scenario became more likely after #118525 started discarding spans that + LL| |// can't be un-expanded back to within the function body. + LL| |// + LL| |// Regression test for "invalid attempt to unwrap a None some_prev", as seen + LL| |// in issues such as #118643 and #118662. + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | affected_function()(); + LL| |} + LL| | + LL| |macro_rules! macro_that_defines_a_function { + LL| | (fn $name:ident () $body:tt) => { + LL| | fn $name () -> impl Fn() $body + LL| | } + LL| |} + LL| | + LL| |macro_that_defines_a_function! { + LL| | fn affected_function() { + LL| 1| || () + LL| | } + LL| |} + diff --git a/tests/coverage/no_spans.rs b/tests/coverage/no_spans.rs new file mode 100644 index 0000000000000..a5234bc6b60d2 --- /dev/null +++ b/tests/coverage/no_spans.rs @@ -0,0 +1,29 @@ +#![feature(coverage_attribute)] +// edition: 2021 + +// If the span extractor can't find any relevant spans for a function, the +// refinement loop will terminate with nothing in its `prev` slot. If the +// subsequent code tries to unwrap `prev`, it will panic. +// +// This scenario became more likely after #118525 started discarding spans that +// can't be un-expanded back to within the function body. +// +// Regression test for "invalid attempt to unwrap a None some_prev", as seen +// in issues such as #118643 and #118662. + +#[coverage(off)] +fn main() { + affected_function()(); +} + +macro_rules! macro_that_defines_a_function { + (fn $name:ident () $body:tt) => { + fn $name () -> impl Fn() $body + } +} + +macro_that_defines_a_function! { + fn affected_function() { + || () + } +} From b378059e6b2573c5356423fa31d184a89a3b6029 Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Mon, 4 Dec 2023 15:52:26 +0000 Subject: [PATCH 11/19] update target feature following LLVM API change LLVM commit https://github.com/llvm/llvm-project/commit/e81796671890b59c110f8e41adc7ca26f8484d20 renamed the `unaligned-scalar-mem` target feature to `fast-unaligned-access`. --- compiler/rustc_codegen_llvm/src/llvm_util.rs | 4 ++++ compiler/rustc_codegen_ssa/src/target_features.rs | 2 +- tests/ui/abi/riscv-discoverability-guidance.rs | 4 ++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index eb69efb0d5952..93cb7327a017e 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -263,6 +263,10 @@ pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> LLVMFeature<'a> { "sve2-bitperm", TargetFeatureFoldStrength::EnableOnly("neon"), ), + // The unaligned-scalar-mem feature was renamed to fast-unaligned-access. + ("riscv32" | "riscv64", "fast-unaligned-access") if get_version().0 <= 17 => { + LLVMFeature::new("unaligned-scalar-mem") + } (_, s) => LLVMFeature::new(s), } } diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index d802816bb7561..c3b8859c77968 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -291,9 +291,9 @@ const RISCV_ALLOWED_FEATURES: &[(&str, Stability)] = &[ ("d", Unstable(sym::riscv_target_feature)), ("e", Unstable(sym::riscv_target_feature)), ("f", Unstable(sym::riscv_target_feature)), + ("fast-unaligned-access", Unstable(sym::riscv_target_feature)), ("m", Stable), ("relax", Unstable(sym::riscv_target_feature)), - ("unaligned-scalar-mem", Unstable(sym::riscv_target_feature)), ("v", Unstable(sym::riscv_target_feature)), ("zba", Stable), ("zbb", Stable), diff --git a/tests/ui/abi/riscv-discoverability-guidance.rs b/tests/ui/abi/riscv-discoverability-guidance.rs index f57fcd6044ffa..361ed8f3d9106 100644 --- a/tests/ui/abi/riscv-discoverability-guidance.rs +++ b/tests/ui/abi/riscv-discoverability-guidance.rs @@ -2,9 +2,9 @@ // revisions: riscv32 riscv64 // // [riscv32] needs-llvm-components: riscv -// [riscv32] compile-flags: --target=riscv32i-unknown-none-elf -C target-feature=-unaligned-scalar-mem --crate-type=rlib +// [riscv32] compile-flags: --target=riscv32i-unknown-none-elf -C target-feature=-fast-unaligned-access --crate-type=rlib // [riscv64] needs-llvm-components: riscv -// [riscv64] compile-flags: --target=riscv64gc-unknown-none-elf -C target-feature=-unaligned-scalar-mem --crate-type=rlib +// [riscv64] compile-flags: --target=riscv64gc-unknown-none-elf -C target-feature=-fast-unaligned-access --crate-type=rlib #![no_core] #![feature( no_core, From 9f0c6f15ce32661e65034898155fcdaa8539201e Mon Sep 17 00:00:00 2001 From: jyn Date: Fri, 8 Dec 2023 11:28:08 -0500 Subject: [PATCH 12/19] Simplify and comment the special-casing for Windows colors --- compiler/rustc_errors/src/emitter.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index ba9cd02a9ece6..62aa8e602af83 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -2674,6 +2674,14 @@ fn from_stderr(color: ColorConfig) -> Destination { } } +/// On Windows, BRIGHT_BLUE is hard to read on black. Use cyan instead. +/// +/// See #36178. +#[cfg(windows)] +const BRIGHT_BLUE: Color = Color::Cyan; +#[cfg(not(windows))] +const BRIGHT_BLUE: Color = Color::Blue; + impl Style { fn color_spec(&self, lvl: Level) -> ColorSpec { let mut spec = ColorSpec::new(); @@ -2688,11 +2696,7 @@ impl Style { Style::LineNumber => { spec.set_bold(true); spec.set_intense(true); - if cfg!(windows) { - spec.set_fg(Some(Color::Cyan)); - } else { - spec.set_fg(Some(Color::Blue)); - } + spec.set_fg(Some(BRIGHT_BLUE)); } Style::Quotation => {} Style::MainHeaderMsg => { @@ -2707,11 +2711,7 @@ impl Style { } Style::UnderlineSecondary | Style::LabelSecondary => { spec.set_bold(true).set_intense(true); - if cfg!(windows) { - spec.set_fg(Some(Color::Cyan)); - } else { - spec.set_fg(Some(Color::Blue)); - } + spec.set_fg(Some(BRIGHT_BLUE)); } Style::HeaderMsg | Style::NoStyle => {} Style::Level(lvl) => { From 96b027f35df93a0fe963c9825b39b248672e18fb Mon Sep 17 00:00:00 2001 From: jyn Date: Fri, 8 Dec 2023 14:09:10 -0500 Subject: [PATCH 13/19] use magenta instead of bold for highlighting according to a poll of gay people in my phone, purple is the most popular color to use for highlighting | color | percentage | | ---------- | ---------- | | bold white | 6% | | blue | 14% | | cyan | 26% | | purple | 37% | | magenta | 17% | unfortunately, purple is not supported by 16-color terminals, which rustc apparently wants to support for some reason. until we require support for full 256-color terms (e.g. by doing the same feature detection as we currently do for urls), we can't use it. instead, i have collapsed the purple votes into magenta on the theory that they're close, and also because magenta is pretty. --- compiler/rustc_errors/src/emitter.rs | 2 +- src/tools/tidy/src/ui_tests.rs | 2 +- tests/ui/error-emitter/highlighting.rs | 23 +++++++++++++++++++ tests/ui/error-emitter/highlighting.stderr | 22 ++++++++++++++++++ .../multiline-multipart-suggestion.rs | 7 +++--- .../multiline-multipart-suggestion.stderr | 14 +++++------ 6 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 tests/ui/error-emitter/highlighting.rs create mode 100644 tests/ui/error-emitter/highlighting.stderr rename tests/ui/{suggestions => error-emitter}/multiline-multipart-suggestion.rs (60%) rename tests/ui/{suggestions => error-emitter}/multiline-multipart-suggestion.stderr (94%) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 62aa8e602af83..3f257fdd9cf27 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -2719,7 +2719,7 @@ impl Style { spec.set_bold(true); } Style::Highlight => { - spec.set_bold(true); + spec.set_bold(true).set_fg(Some(Color::Magenta)); } } spec diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 40149f8f1c3b6..dfa386b49de7c 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -11,7 +11,7 @@ use std::path::{Path, PathBuf}; const ENTRY_LIMIT: usize = 900; // FIXME: The following limits should be reduced eventually. const ISSUES_ENTRY_LIMIT: usize = 1852; -const ROOT_ENTRY_LIMIT: usize = 866; +const ROOT_ENTRY_LIMIT: usize = 867; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ "rs", // test source files diff --git a/tests/ui/error-emitter/highlighting.rs b/tests/ui/error-emitter/highlighting.rs new file mode 100644 index 0000000000000..f7c15100fed78 --- /dev/null +++ b/tests/ui/error-emitter/highlighting.rs @@ -0,0 +1,23 @@ +// Make sure "highlighted" code is colored purple + +// compile-flags: --error-format=human --color=always +// error-pattern:for<'a>  +// edition:2018 + +use core::pin::Pin; +use core::future::Future; +use core::any::Any; + +fn query(_: fn(Box<(dyn Any + Send + '_)>) -> Pin, String>> + Send + 'static +)>>) {} + +fn wrapped_fn<'a>(_: Box<(dyn Any + Send)>) -> Pin, String>> + Send + 'static +)>> { + Box::pin(async { Err("nope".into()) }) +} + +fn main() { + query(wrapped_fn); +} diff --git a/tests/ui/error-emitter/highlighting.stderr b/tests/ui/error-emitter/highlighting.stderr new file mode 100644 index 0000000000000..12a1caa6ef315 --- /dev/null +++ b/tests/ui/error-emitter/highlighting.stderr @@ -0,0 +1,22 @@ +error[E0308]: mismatched types + --> $DIR/highlighting.rs:22:11 + | +LL |  query(wrapped_fn); + |  ----- ^^^^^^^^^^ one type is more general than the other + |  | + |  arguments to this function are incorrect + | + = note: expected fn pointer `for<'a> fn(Box<(dyn Any + Send + 'a)>) -> Pin<_>` + found fn item `fn(Box<(dyn Any + Send + 'static)>) -> Pin<_> {wrapped_fn}` +note: function defined here + --> $DIR/highlighting.rs:11:4 + | +LL | fn query(_: fn(Box<(dyn Any + Send + '_)>) -> Pin, String>> + Send + 'static +LL | | )>>) {} + | |___- + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/suggestions/multiline-multipart-suggestion.rs b/tests/ui/error-emitter/multiline-multipart-suggestion.rs similarity index 60% rename from tests/ui/suggestions/multiline-multipart-suggestion.rs rename to tests/ui/error-emitter/multiline-multipart-suggestion.rs index 77d0322d05fcf..5532fe3d6f779 100644 --- a/tests/ui/suggestions/multiline-multipart-suggestion.rs +++ b/tests/ui/error-emitter/multiline-multipart-suggestion.rs @@ -1,18 +1,19 @@ // compile-flags: --error-format=human --color=always +// error-pattern: missing lifetime specifier // ignore-windows -fn short(foo_bar: &Vec<&i32>) -> &i32 { //~ ERROR missing lifetime specifier +fn short(foo_bar: &Vec<&i32>) -> &i32 { &12 } -fn long( //~ ERROR missing lifetime specifier +fn long( foo_bar: &Vec<&i32>, something_very_long_so_that_the_line_will_wrap_around__________: i32, ) -> &i32 { &12 } -fn long2( //~ ERROR missing lifetime specifier +fn long2( foo_bar: &Vec<&i32>) -> &i32 { &12 } diff --git a/tests/ui/suggestions/multiline-multipart-suggestion.stderr b/tests/ui/error-emitter/multiline-multipart-suggestion.stderr similarity index 94% rename from tests/ui/suggestions/multiline-multipart-suggestion.stderr rename to tests/ui/error-emitter/multiline-multipart-suggestion.stderr index 045a86b4f541f..7f418fe8ad1eb 100644 --- a/tests/ui/suggestions/multiline-multipart-suggestion.stderr +++ b/tests/ui/error-emitter/multiline-multipart-suggestion.stderr @@ -1,17 +1,17 @@ error[E0106]: missing lifetime specifier - --> $DIR/multiline-multipart-suggestion.rs:4:34 + --> $DIR/multiline-multipart-suggestion.rs:5:34  | -LL | fn short(foo_bar: &Vec<&i32>) -> &i32 { +LL | fn short(foo_bar: &Vec<&i32>) -> &i32 {  |  ---------- ^ expected named lifetime parameter  |  = help: this function's return type contains a borrowed value, but the signature does not say which one of `foo_bar`'s 2 lifetimes it is borrowed from help: consider introducing a named lifetime parameter  | -LL | fn short<'a>(foo_bar: &'a Vec<&'a i32>) -> &'a i32 { +LL | fn short<'a>(foo_bar: &'a Vec<&'a i32>) -> &'a i32 {  | ++++ ++ ++ ++ error[E0106]: missing lifetime specifier - --> $DIR/multiline-multipart-suggestion.rs:11:6 + --> $DIR/multiline-multipart-suggestion.rs:12:6  | LL |  foo_bar: &Vec<&i32>,  |  ---------- @@ -22,14 +22,14 @@  = help: this function's return type contains a borrowed value, but the signature does not say which one of `foo_bar`'s 2 lifetimes it is borrowed from help: consider introducing a named lifetime parameter  | -LL ~ fn long<'a>( +LL ~ fn long<'a>( LL ~  foo_bar: &'a Vec<&'a i32>, LL |  something_very_long_so_that_the_line_will_wrap_around__________: i32, LL ~ ) -> &'a i32 {  | error[E0106]: missing lifetime specifier - --> $DIR/multiline-multipart-suggestion.rs:16:29 + --> $DIR/multiline-multipart-suggestion.rs:17:29  | LL |  foo_bar: &Vec<&i32>) -> &i32 {  |  ---------- ^ expected named lifetime parameter @@ -37,7 +37,7 @@  = help: this function's return type contains a borrowed value, but the signature does not say which one of `foo_bar`'s 2 lifetimes it is borrowed from help: consider introducing a named lifetime parameter  | -LL ~ fn long2<'a>( +LL ~ fn long2<'a>( LL ~  foo_bar: &'a Vec<&'a i32>) -> &'a i32 {  | From 287c77e9212d6ad4e806c000eb6a4ee8e0ce462f Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 1 Dec 2023 17:13:01 -0300 Subject: [PATCH 14/19] Add tests related to normalization in implied bounds --- tests/ui/implied-bounds/from-trait-impl.rs | 24 +++++++++++++++ .../normalization-nested.lifetime.stderr | 29 ++++++++++++++----- .../ui/implied-bounds/normalization-nested.rs | 4 ++- ...lization-preserve-equality.borrowck.stderr | 28 ++++++++++++++++++ .../normalization-preserve-equality.rs | 28 ++++++++++++++++++ 5 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 tests/ui/implied-bounds/from-trait-impl.rs create mode 100644 tests/ui/implied-bounds/normalization-preserve-equality.borrowck.stderr create mode 100644 tests/ui/implied-bounds/normalization-preserve-equality.rs diff --git a/tests/ui/implied-bounds/from-trait-impl.rs b/tests/ui/implied-bounds/from-trait-impl.rs new file mode 100644 index 0000000000000..d13fddd9b8d41 --- /dev/null +++ b/tests/ui/implied-bounds/from-trait-impl.rs @@ -0,0 +1,24 @@ +// check-pass +// known-bug: #109628 + +trait Trait { + type Assoc; +} + +impl Trait for (X,) { + type Assoc = (); +} + +struct Foo(T) +where + T::Assoc: Clone; // any predicate using `T::Assoc` works here + +fn func1(foo: Foo<(&str,)>) { + let _: &'static str = foo.0.0; +} + +trait TestTrait {} + +impl TestTrait for [Foo<(X,)>; 1] {} + +fn main() {} diff --git a/tests/ui/implied-bounds/normalization-nested.lifetime.stderr b/tests/ui/implied-bounds/normalization-nested.lifetime.stderr index abffee57a0f09..e020230d86a48 100644 --- a/tests/ui/implied-bounds/normalization-nested.lifetime.stderr +++ b/tests/ui/implied-bounds/normalization-nested.lifetime.stderr @@ -1,11 +1,11 @@ error[E0759]: `fn` parameter has lifetime `'x` but it needs to satisfy a `'static` lifetime requirement - --> $DIR/normalization-nested.rs:35:20 + --> $DIR/normalization-nested.rs:35:28 | -LL | pub fn test<'x>(_: Map>, s: &'x str) -> &'static str { - | ^^^^^^^^^^^^^^^^ - | | - | this data with lifetime `'x`... - | ...is used and required to live as long as `'static` here +LL | pub fn test_wfcheck<'x>(_: Map>) {} + | ^^^^^^^^^^^^^^^^ + | | + | this data with lifetime `'x`... + | ...is used and required to live as long as `'static` here | note: `'static` lifetime requirement introduced by this bound --> $DIR/normalization-nested.rs:33:14 @@ -13,6 +13,21 @@ note: `'static` lifetime requirement introduced by this bound LL | I::Item: 'static; | ^^^^^^^ -error: aborting due to 1 previous error +error[E0759]: `fn` parameter has lifetime `'x` but it needs to satisfy a `'static` lifetime requirement + --> $DIR/normalization-nested.rs:37:29 + | +LL | pub fn test_borrowck<'x>(_: Map>, s: &'x str) -> &'static str { + | ^^^^^^^^^^^^^^^^ + | | + | this data with lifetime `'x`... + | ...is used and required to live as long as `'static` here + | +note: `'static` lifetime requirement introduced by this bound + --> $DIR/normalization-nested.rs:33:14 + | +LL | I::Item: 'static; + | ^^^^^^^ + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0759`. diff --git a/tests/ui/implied-bounds/normalization-nested.rs b/tests/ui/implied-bounds/normalization-nested.rs index 5f1cbb3f69779..87903783a678d 100644 --- a/tests/ui/implied-bounds/normalization-nested.rs +++ b/tests/ui/implied-bounds/normalization-nested.rs @@ -32,7 +32,9 @@ where I: Iter, I::Item: 'static; -pub fn test<'x>(_: Map>, s: &'x str) -> &'static str { +pub fn test_wfcheck<'x>(_: Map>) {} + +pub fn test_borrowck<'x>(_: Map>, s: &'x str) -> &'static str { s } diff --git a/tests/ui/implied-bounds/normalization-preserve-equality.borrowck.stderr b/tests/ui/implied-bounds/normalization-preserve-equality.borrowck.stderr new file mode 100644 index 0000000000000..96c76ca9ac311 --- /dev/null +++ b/tests/ui/implied-bounds/normalization-preserve-equality.borrowck.stderr @@ -0,0 +1,28 @@ +error: lifetime may not live long enough + --> $DIR/normalization-preserve-equality.rs:24:1 + | +LL | fn test_borrowck<'a, 'b>(_: ( as Trait>::Ty, Equal<'a, 'b>)) { + | ^^^^^^^^^^^^^^^^^--^^--^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | | | + | | | lifetime `'b` defined here + | | lifetime `'a` defined here + | requires that `'a` must outlive `'b` + | + = help: consider adding the following bound: `'a: 'b` + +error: lifetime may not live long enough + --> $DIR/normalization-preserve-equality.rs:24:1 + | +LL | fn test_borrowck<'a, 'b>(_: ( as Trait>::Ty, Equal<'a, 'b>)) { + | ^^^^^^^^^^^^^^^^^--^^--^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | | | + | | | lifetime `'b` defined here + | | lifetime `'a` defined here + | requires that `'b` must outlive `'a` + | + = help: consider adding the following bound: `'b: 'a` + +help: `'a` and `'b` must be the same: replace one with the other + +error: aborting due to 2 previous errors + diff --git a/tests/ui/implied-bounds/normalization-preserve-equality.rs b/tests/ui/implied-bounds/normalization-preserve-equality.rs new file mode 100644 index 0000000000000..557c171e515a3 --- /dev/null +++ b/tests/ui/implied-bounds/normalization-preserve-equality.rs @@ -0,0 +1,28 @@ +// Both revisions should pass. `borrowck` revision is a bug! +// +// revisions: wfcheck borrowck +// [wfcheck] check-pass +// [borrowck] check-fail +// [borrowck] known-bug: #106569 + +struct Equal<'a, 'b>(&'a &'b (), &'b &'a ()); // implies 'a == 'b + +trait Trait { + type Ty; +} + +impl<'x> Trait for Equal<'x, 'x> { + type Ty = (); +} + +trait WfCheckTrait {} + +#[cfg(wfcheck)] +impl<'a, 'b> WfCheckTrait for ( as Trait>::Ty, Equal<'a, 'b>) {} + +#[cfg(borrowck)] +fn test_borrowck<'a, 'b>(_: ( as Trait>::Ty, Equal<'a, 'b>)) { + let _ = None::>; +} + +fn main() {} From 8361a7288e42d303890a63092fe5fd276819c79d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 8 Dec 2023 21:38:00 +0000 Subject: [PATCH 15/19] Introduce closure_id method on CoroutineKind --- compiler/rustc_ast/src/ast.rs | 8 +++++++ compiler/rustc_ast_lowering/src/item.rs | 5 +---- compiler/rustc_lint/src/early.rs | 10 ++------- compiler/rustc_resolve/src/def_collector.rs | 25 +++++++++++---------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 0b06af223653c..190fae9565210 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2450,6 +2450,14 @@ impl CoroutineKind { matches!(self, CoroutineKind::Gen { .. }) } + pub fn closure_id(self) -> NodeId { + match self { + CoroutineKind::Async { closure_id, .. } + | CoroutineKind::Gen { closure_id, .. } + | CoroutineKind::AsyncGen { closure_id, .. } => closure_id, + } + } + /// In this case this is an `async` or `gen` return, the `NodeId` for the generated `impl Trait` /// item. pub fn return_id(self) -> (NodeId, Span) { diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 9d1f2684c394d..a4effb99e71eb 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -1035,10 +1035,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let (Some(coroutine_kind), Some(body)) = (coroutine_kind, body) else { return self.lower_fn_body_block(span, decl, body); }; - // FIXME(gen_blocks): Introduce `closure_id` method and remove ALL destructuring. - let (CoroutineKind::Async { closure_id, .. } - | CoroutineKind::Gen { closure_id, .. } - | CoroutineKind::AsyncGen { closure_id, .. }) = coroutine_kind; + let closure_id = coroutine_kind.closure_id(); self.lower_body(|this| { let mut parameters: Vec> = Vec::new(); diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index 80c6feaa26936..71c9aa79e534a 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -163,10 +163,7 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> // it does not have a corresponding AST node if let ast_visit::FnKind::Fn(_, _, sig, _, _, _) = fk { if let Some(coro_kind) = sig.header.coroutine_kind { - let (ast::CoroutineKind::Async { closure_id, .. } - | ast::CoroutineKind::Gen { closure_id, .. } - | ast::CoroutineKind::AsyncGen { closure_id, .. }) = coro_kind; - self.check_id(closure_id); + self.check_id(coro_kind.closure_id()); } } } @@ -228,10 +225,7 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> ast::ExprKind::Closure(box ast::Closure { coroutine_kind: Some(coro_kind), .. }) => { - let (ast::CoroutineKind::Async { closure_id, .. } - | ast::CoroutineKind::Gen { closure_id, .. } - | ast::CoroutineKind::AsyncGen { closure_id, .. }) = coro_kind; - self.check_id(closure_id); + self.check_id(coro_kind.closure_id()); } _ => {} } diff --git a/compiler/rustc_resolve/src/def_collector.rs b/compiler/rustc_resolve/src/def_collector.rs index 186dd28b142e6..02553d5007155 100644 --- a/compiler/rustc_resolve/src/def_collector.rs +++ b/compiler/rustc_resolve/src/def_collector.rs @@ -157,11 +157,7 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> { fn visit_fn(&mut self, fn_kind: FnKind<'a>, span: Span, _: NodeId) { if let FnKind::Fn(_, _, sig, _, generics, body) = fn_kind { match sig.header.coroutine_kind { - Some( - CoroutineKind::Async { closure_id, .. } - | CoroutineKind::Gen { closure_id, .. } - | CoroutineKind::AsyncGen { closure_id, .. }, - ) => { + Some(coroutine_kind) => { self.visit_generics(generics); // For async functions, we need to create their inner defs inside of a @@ -176,8 +172,12 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> { // then the closure_def will never be used, and we should avoid generating a // def-id for it. if let Some(body) = body { - let closure_def = - self.create_def(closure_id, kw::Empty, DefKind::Closure, span); + let closure_def = self.create_def( + coroutine_kind.closure_id(), + kw::Empty, + DefKind::Closure, + span, + ); self.with_parent(closure_def, |this| this.visit_block(body)); } return; @@ -289,11 +289,12 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> { // we must create two defs. let closure_def = self.create_def(expr.id, kw::Empty, DefKind::Closure, expr.span); match closure.coroutine_kind { - Some( - CoroutineKind::Async { closure_id, .. } - | CoroutineKind::Gen { closure_id, .. } - | CoroutineKind::AsyncGen { closure_id, .. }, - ) => self.create_def(closure_id, kw::Empty, DefKind::Closure, expr.span), + Some(coroutine_kind) => self.create_def( + coroutine_kind.closure_id(), + kw::Empty, + DefKind::Closure, + expr.span, + ), None => closure_def, } } From d5dcd85376925f5f3d5fdbdcc9467165fe2c0b18 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 8 Dec 2023 21:46:08 +0000 Subject: [PATCH 16/19] More nits --- compiler/rustc_ast_lowering/src/lib.rs | 3 ++- .../src/traits/error_reporting/suggestions.rs | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 753650f732410..0c71165deedd0 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -177,7 +177,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } else { [sym::gen_future].into() }, - // FIXME(gen_blocks): how does `closure_track_caller` + // FIXME(gen_blocks): how does `closure_track_caller`/`async_fn_track_caller` + // interact with `gen`/`async gen` blocks allow_async_iterator: [sym::gen_future, sym::async_iterator].into(), generics_def_id_map: Default::default(), host_param_id: None, diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 7bf37cf79806a..95ffd07e39783 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -3144,10 +3144,9 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { let what = match self.tcx.coroutine_kind(coroutine_def_id) { None | Some(hir::CoroutineKind::Coroutine) - | Some(hir::CoroutineKind::Gen(_)) - // FIXME(gen_blocks): This could be yield or await... - | Some(hir::CoroutineKind::AsyncGen(_)) => "yield", + | Some(hir::CoroutineKind::Gen(_)) => "yield", Some(hir::CoroutineKind::Async(..)) => "await", + Some(hir::CoroutineKind::AsyncGen(_)) => "yield`/`await", }; err.note(format!( "all values live across `{what}` must have a statically known size" From 384a49edd0cddba275e6be73acbb9a7c37ec03b2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 8 Dec 2023 21:46:30 +0000 Subject: [PATCH 17/19] Rename some more coro_kind -> coroutine_kind --- compiler/rustc_ast_passes/src/ast_validation.rs | 4 ++-- compiler/rustc_builtin_macros/src/test.rs | 4 ++-- compiler/rustc_lint/src/early.rs | 9 +++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 0644c4cd6be4c..1f9bc09f5f7f3 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -1271,11 +1271,11 @@ impl<'a> Visitor<'a> for AstValidator<'a> { // Functions cannot both be `const async` or `const gen` if let Some(&FnHeader { constness: Const::Yes(cspan), - coroutine_kind: Some(coro_kind), + coroutine_kind: Some(coroutine_kind), .. }) = fk.header() { - let aspan = match coro_kind { + let aspan = match coroutine_kind { CoroutineKind::Async { span: aspan, .. } | CoroutineKind::Gen { span: aspan, .. } | CoroutineKind::AsyncGen { span: aspan, .. } => aspan, diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs index 794be25955d63..e5b274304e7f2 100644 --- a/compiler/rustc_builtin_macros/src/test.rs +++ b/compiler/rustc_builtin_macros/src/test.rs @@ -541,8 +541,8 @@ fn check_test_signature( return Err(sd.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "unsafe" })); } - if let Some(coro_kind) = f.sig.header.coroutine_kind { - match coro_kind { + if let Some(coroutine_kind) = f.sig.header.coroutine_kind { + match coroutine_kind { ast::CoroutineKind::Async { span, .. } => { return Err(sd.emit_err(errors::TestBadFn { span: i.span, diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index 71c9aa79e534a..4c7f9eeff8c28 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -162,8 +162,8 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> // Explicitly check for lints associated with 'closure_id', since // it does not have a corresponding AST node if let ast_visit::FnKind::Fn(_, _, sig, _, _, _) = fk { - if let Some(coro_kind) = sig.header.coroutine_kind { - self.check_id(coro_kind.closure_id()); + if let Some(coroutine_kind) = sig.header.coroutine_kind { + self.check_id(coroutine_kind.closure_id()); } } } @@ -223,9 +223,10 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> // it does not have a corresponding AST node match e.kind { ast::ExprKind::Closure(box ast::Closure { - coroutine_kind: Some(coro_kind), .. + coroutine_kind: Some(coroutine_kind), + .. }) => { - self.check_id(coro_kind.closure_id()); + self.check_id(coroutine_kind.closure_id()); } _ => {} } From e9878125216e660601182dcadbc1c86910333a64 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 8 Dec 2023 22:25:12 +0000 Subject: [PATCH 18/19] Make async generators fused by default --- compiler/rustc_mir_transform/src/coroutine.rs | 44 ++++++++++++++----- tests/ui/coroutine/async_gen_fn_iter.rs | 4 ++ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index 2b591abb05d66..737fb6bf61229 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -252,15 +252,15 @@ struct TransformVisitor<'tcx> { impl<'tcx> TransformVisitor<'tcx> { fn insert_none_ret_block(&self, body: &mut Body<'tcx>) -> BasicBlock { - assert!(matches!(self.coroutine_kind, CoroutineKind::Gen(_))); - let block = BasicBlock::new(body.basic_blocks.len()); let source_info = SourceInfo::outermost(body.span); - let option_def_id = self.tcx.require_lang_item(LangItem::Option, None); - let statements = vec![Statement { - kind: StatementKind::Assign(Box::new(( - Place::return_place(), + let none_value = match self.coroutine_kind { + CoroutineKind::Async(_) => span_bug!(body.span, "`Future`s are not fused inherently"), + CoroutineKind::Coroutine => span_bug!(body.span, "`Coroutine`s cannot be fused"), + // `gen` continues return `None` + CoroutineKind::Gen(_) => { + let option_def_id = self.tcx.require_lang_item(LangItem::Option, None); Rvalue::Aggregate( Box::new(AggregateKind::Adt( option_def_id, @@ -270,8 +270,29 @@ impl<'tcx> TransformVisitor<'tcx> { None, )), IndexVec::new(), - ), - ))), + ) + } + // `async gen` continues to return `Poll::Ready(None)` + CoroutineKind::AsyncGen(_) => { + let ty::Adt(_poll_adt, args) = *self.old_yield_ty.kind() else { bug!() }; + let ty::Adt(_option_adt, args) = *args.type_at(0).kind() else { bug!() }; + let yield_ty = args.type_at(0); + Rvalue::Use(Operand::Constant(Box::new(ConstOperand { + span: source_info.span, + const_: Const::Unevaluated( + UnevaluatedConst::new( + self.tcx.require_lang_item(LangItem::AsyncGenFinished, None), + self.tcx.mk_args(&[yield_ty.into()]), + ), + self.old_yield_ty, + ), + user_ty: None, + }))) + } + }; + + let statements = vec![Statement { + kind: StatementKind::Assign(Box::new((Place::return_place(), none_value))), source_info, }]; @@ -1393,11 +1414,12 @@ fn create_coroutine_resume_function<'tcx>( if can_return { let block = match coroutine_kind { - // FIXME(gen_blocks): Should `async gen` yield `None` when resumed once again? - CoroutineKind::Async(_) | CoroutineKind::AsyncGen(_) | CoroutineKind::Coroutine => { + CoroutineKind::Async(_) | CoroutineKind::Coroutine => { insert_panic_block(tcx, body, ResumedAfterReturn(coroutine_kind)) } - CoroutineKind::Gen(_) => transform.insert_none_ret_block(body), + CoroutineKind::AsyncGen(_) | CoroutineKind::Gen(_) => { + transform.insert_none_ret_block(body) + } }; cases.insert(1, (RETURNED, block)); } diff --git a/tests/ui/coroutine/async_gen_fn_iter.rs b/tests/ui/coroutine/async_gen_fn_iter.rs index 6f8f3feb87e92..4fa29e1095a14 100644 --- a/tests/ui/coroutine/async_gen_fn_iter.rs +++ b/tests/ui/coroutine/async_gen_fn_iter.rs @@ -33,6 +33,10 @@ async fn async_main() { assert_eq!(iter.as_mut().next().await, Some(2)); assert_eq!(iter.as_mut().next().await, Some(3)); assert_eq!(iter.as_mut().next().await, None); + + // Test that the iterator is fused and does not panic + assert_eq!(iter.as_mut().next().await, None); + assert_eq!(iter.as_mut().next().await, None); } // ------------------------------------------------------------------------- // From 7079adb226ea27a573715da6a789e8c5cb2609d9 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 8 Dec 2023 18:20:14 -0300 Subject: [PATCH 19/19] Add Bevy related test cases --- tests/ui/implied-bounds/auxiliary/bevy_ecs.rs | 18 +++++++++ tests/ui/implied-bounds/bevy_world_query.rs | 11 ++++++ tests/ui/implied-bounds/gluon_salsa.rs | 31 ++++++++++++++++ tests/ui/implied-bounds/sod_service_chain.rs | 37 +++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 tests/ui/implied-bounds/auxiliary/bevy_ecs.rs create mode 100644 tests/ui/implied-bounds/bevy_world_query.rs create mode 100644 tests/ui/implied-bounds/gluon_salsa.rs create mode 100644 tests/ui/implied-bounds/sod_service_chain.rs diff --git a/tests/ui/implied-bounds/auxiliary/bevy_ecs.rs b/tests/ui/implied-bounds/auxiliary/bevy_ecs.rs new file mode 100644 index 0000000000000..b373d39f4d940 --- /dev/null +++ b/tests/ui/implied-bounds/auxiliary/bevy_ecs.rs @@ -0,0 +1,18 @@ +// Related to Bevy regression #118553 + +pub trait WorldQuery {} +impl WorldQuery for &u8 {} + +pub struct Query(Q); + +pub trait SystemParam { + type State; +} +impl SystemParam for Query { + type State = (); + // `Q: 'static` is required because we need the TypeId of Q ... +} + +pub struct ParamSet(T) +where + T::State: Sized; diff --git a/tests/ui/implied-bounds/bevy_world_query.rs b/tests/ui/implied-bounds/bevy_world_query.rs new file mode 100644 index 0000000000000..f8e64632676d7 --- /dev/null +++ b/tests/ui/implied-bounds/bevy_world_query.rs @@ -0,0 +1,11 @@ +// aux-crate:bevy_ecs=bevy_ecs.rs +// check-pass +// Related to Bevy regression #118553 + +extern crate bevy_ecs; + +use bevy_ecs::*; + +fn handler<'a>(_: ParamSet>) {} + +fn main() {} diff --git a/tests/ui/implied-bounds/gluon_salsa.rs b/tests/ui/implied-bounds/gluon_salsa.rs new file mode 100644 index 0000000000000..98951af8ac2da --- /dev/null +++ b/tests/ui/implied-bounds/gluon_salsa.rs @@ -0,0 +1,31 @@ +// check-pass +// Related to Bevy regression #118553 + +pub trait QueryBase { + type Db; +} + +pub trait AsyncQueryFunction<'f>: // 'f is important + QueryBase>::SendDb> // bound is important +{ + type SendDb; +} + +pub struct QueryTable<'me, Q, DB> { + _q: Option, + _db: Option, + _marker: Option<&'me ()>, +} + +impl<'me, Q> QueryTable<'me, Q, ::Db> +// projection is important +// ^^^ removing 'me (and in QueryTable) gives a different error +where + Q: for<'f> AsyncQueryFunction<'f>, +{ + pub fn get_async<'a>(&'a mut self) { + panic!(); + } +} + +fn main() {} diff --git a/tests/ui/implied-bounds/sod_service_chain.rs b/tests/ui/implied-bounds/sod_service_chain.rs new file mode 100644 index 0000000000000..f45ced71f757b --- /dev/null +++ b/tests/ui/implied-bounds/sod_service_chain.rs @@ -0,0 +1,37 @@ +// check-pass +// Related to crater regressions on #118553 + +pub trait Debug {} + +pub trait Service { + type Input; + type Output; + type Error; +} + +pub struct ServiceChain { + prev: P, + service: S, +} +impl> Service for ServiceChain +where + P::Error: 'static, + S::Error: 'static, +{ + type Input = P::Input; + type Output = S::Output; + type Error = (); +} + +pub struct ServiceChainBuilder> { + chain: ServiceChain, +} +impl> ServiceChainBuilder { + pub fn next>( + self, + ) -> ServiceChainBuilder, NS> { + panic!(); + } +} + +fn main() {}