From 72e22554caa473df6af10f0ab6c3c7c7c5ef8841 Mon Sep 17 00:00:00 2001 From: Augie Fackler Date: Wed, 17 Jul 2024 10:07:17 -0400 Subject: [PATCH 01/20] cleanup: remove support for 3DNow! cpu features In llvm/llvm-project@f0eb5587ceeb641445b64cb264c822b4751de04a all support for 3DNow! intrinsics and instructions were removed. Per the commit message there, only AMD chips between 1998 and 2011 or so actually supported these instructions, and they were effectively replaced by SSE which was available on many more chips. I'd be very surprised if anyone had ever used these from Rust. --- compiler/rustc_target/src/spec/targets/x86_64_unknown_none.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_target/src/spec/targets/x86_64_unknown_none.rs b/compiler/rustc_target/src/spec/targets/x86_64_unknown_none.rs index 4c67fa0f7aa64..549706998d463 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_unknown_none.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_unknown_none.rs @@ -18,9 +18,7 @@ pub fn target() -> Target { relro_level: RelroLevel::Full, linker_flavor: LinkerFlavor::Gnu(Cc::No, Lld::Yes), linker: Some("rust-lld".into()), - features: - "-mmx,-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-3dnow,-3dnowa,-avx,-avx2,+soft-float" - .into(), + features: "-mmx,-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-avx,-avx2,+soft-float".into(), supported_sanitizers: SanitizerSet::KCFI | SanitizerSet::KERNELADDRESS, disable_redzone: true, panic_strategy: PanicStrategy::Abort, From a47ca1913469e574369e6ddc28f124e5c9e9e13a Mon Sep 17 00:00:00 2001 From: Oneirical Date: Mon, 8 Jul 2024 13:42:46 -0400 Subject: [PATCH 02/20] rewrite extern-fn-with-extern-types to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../extern-fn-with-extern-types/Makefile | 6 ------ .../extern-fn-with-extern-types/rmake.rs | 16 ++++++++++++++++ 3 files changed, 16 insertions(+), 7 deletions(-) delete mode 100644 tests/run-make/extern-fn-with-extern-types/Makefile create mode 100644 tests/run-make/extern-fn-with-extern-types/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 2e26f9344b899..e016d3f11398f 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -29,7 +29,6 @@ run-make/extern-fn-generic/Makefile run-make/extern-fn-mangle/Makefile run-make/extern-fn-reachable/Makefile run-make/extern-fn-struct-passing-abi/Makefile -run-make/extern-fn-with-extern-types/Makefile run-make/extern-fn-with-packed-struct/Makefile run-make/extern-fn-with-union/Makefile run-make/extern-multiple-copies/Makefile diff --git a/tests/run-make/extern-fn-with-extern-types/Makefile b/tests/run-make/extern-fn-with-extern-types/Makefile deleted file mode 100644 index 07ec503aaaea5..0000000000000 --- a/tests/run-make/extern-fn-with-extern-types/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,ctest) - $(RUSTC) test.rs - $(call RUN,test) || exit 1 diff --git a/tests/run-make/extern-fn-with-extern-types/rmake.rs b/tests/run-make/extern-fn-with-extern-types/rmake.rs new file mode 100644 index 0000000000000..02521ae2cdb1d --- /dev/null +++ b/tests/run-make/extern-fn-with-extern-types/rmake.rs @@ -0,0 +1,16 @@ +// This test checks the functionality of foreign function interface (FFI) where Rust +// must call upon a C library defining functions, where these functions also use custom +// types defined by the C file. In addition to compilation being successful, the binary +// should also successfully execute. +// See https://github.com/rust-lang/rust/pull/44295 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("ctest"); + rustc().input("test.rs").run(); + run("test"); +} From a795d8998d6715044a4e28ffda90a251a780eae1 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Mon, 8 Jul 2024 13:54:04 -0400 Subject: [PATCH 03/20] rewrite and rename issue-25581 --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/extern-fn-slice-no-ice/rmake.rs | 17 +++++++++++++++++ .../test.c | 0 .../test.rs | 0 tests/run-make/issue-25581/Makefile | 6 ------ 5 files changed, 17 insertions(+), 7 deletions(-) create mode 100644 tests/run-make/extern-fn-slice-no-ice/rmake.rs rename tests/run-make/{issue-25581 => extern-fn-slice-no-ice}/test.c (100%) rename tests/run-make/{issue-25581 => extern-fn-slice-no-ice}/test.rs (100%) delete mode 100644 tests/run-make/issue-25581/Makefile diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index e016d3f11398f..5a7fe935e7096 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -44,7 +44,6 @@ run-make/issue-107094/Makefile run-make/issue-14698/Makefile run-make/issue-15460/Makefile run-make/issue-22131/Makefile -run-make/issue-25581/Makefile run-make/issue-26006/Makefile run-make/issue-28595/Makefile run-make/issue-33329/Makefile diff --git a/tests/run-make/extern-fn-slice-no-ice/rmake.rs b/tests/run-make/extern-fn-slice-no-ice/rmake.rs new file mode 100644 index 0000000000000..1f1bbd331273a --- /dev/null +++ b/tests/run-make/extern-fn-slice-no-ice/rmake.rs @@ -0,0 +1,17 @@ +// Slices were broken when implicated in foreign-function interface (FFI) with +// a C library, with something as simple as measuring the length or returning +// an item at a certain index of a slice would cause an internal compiler error (ICE). +// This was fixed in #25653, and this test checks that slices in Rust-C FFI can be part +// of a program that compiles and executes successfully. +// See https://github.com/rust-lang/rust/issues/25581 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("test"); + rustc().input("test.rs").run(); + run("test"); +} diff --git a/tests/run-make/issue-25581/test.c b/tests/run-make/extern-fn-slice-no-ice/test.c similarity index 100% rename from tests/run-make/issue-25581/test.c rename to tests/run-make/extern-fn-slice-no-ice/test.c diff --git a/tests/run-make/issue-25581/test.rs b/tests/run-make/extern-fn-slice-no-ice/test.rs similarity index 100% rename from tests/run-make/issue-25581/test.rs rename to tests/run-make/extern-fn-slice-no-ice/test.rs diff --git a/tests/run-make/issue-25581/Makefile b/tests/run-make/issue-25581/Makefile deleted file mode 100644 index 3cbbf3839969f..0000000000000 --- a/tests/run-make/issue-25581/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,test) - $(RUSTC) test.rs - $(call RUN,test) || exit 1 From d6a3f65db099716fb03f056716d4114948bf7087 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Mon, 8 Jul 2024 14:03:04 -0400 Subject: [PATCH 04/20] rewrite extern-fn-struct-passing-abi to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../extern-fn-struct-passing-abi/Makefile | 6 ------ .../extern-fn-struct-passing-abi/rmake.rs | 16 ++++++++++++++++ 3 files changed, 16 insertions(+), 7 deletions(-) delete mode 100644 tests/run-make/extern-fn-struct-passing-abi/Makefile create mode 100644 tests/run-make/extern-fn-struct-passing-abi/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 5a7fe935e7096..645f489cb39c8 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -28,7 +28,6 @@ run-make/extern-fn-explicit-align/Makefile run-make/extern-fn-generic/Makefile run-make/extern-fn-mangle/Makefile run-make/extern-fn-reachable/Makefile -run-make/extern-fn-struct-passing-abi/Makefile run-make/extern-fn-with-packed-struct/Makefile run-make/extern-fn-with-union/Makefile run-make/extern-multiple-copies/Makefile diff --git a/tests/run-make/extern-fn-struct-passing-abi/Makefile b/tests/run-make/extern-fn-struct-passing-abi/Makefile deleted file mode 100644 index 3cbbf3839969f..0000000000000 --- a/tests/run-make/extern-fn-struct-passing-abi/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,test) - $(RUSTC) test.rs - $(call RUN,test) || exit 1 diff --git a/tests/run-make/extern-fn-struct-passing-abi/rmake.rs b/tests/run-make/extern-fn-struct-passing-abi/rmake.rs new file mode 100644 index 0000000000000..5c4ddb7f58e65 --- /dev/null +++ b/tests/run-make/extern-fn-struct-passing-abi/rmake.rs @@ -0,0 +1,16 @@ +// Functions with more than 6 arguments using foreign function interfaces (FFI) with C libraries +// would have their arguments unexpectedly swapped, causing unexpected behaviour in Rust-C FFI +// programs. This test compiles and executes Rust code with bulky functions of up to 7 arguments +// and uses assertions to check for unexpected swaps. +// See https://github.com/rust-lang/rust/issues/25594 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("test"); + rustc().input("test.rs").run(); + run("test"); +} From 98454ece33fc282fbcb27b8f56764347d247798d Mon Sep 17 00:00:00 2001 From: Oneirical Date: Mon, 8 Jul 2024 14:15:01 -0400 Subject: [PATCH 05/20] rewrite longjmp-across-rust to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/longjmp-across-rust/Makefile | 6 ------ tests/run-make/longjmp-across-rust/rmake.rs | 18 ++++++++++++++++++ 3 files changed, 18 insertions(+), 7 deletions(-) delete mode 100644 tests/run-make/longjmp-across-rust/Makefile create mode 100644 tests/run-make/longjmp-across-rust/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 645f489cb39c8..2f2af6b8bc1dc 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -65,7 +65,6 @@ run-make/link-path-order/Makefile run-make/linkage-attr-on-static/Makefile run-make/long-linker-command-lines-cmd-exe/Makefile run-make/long-linker-command-lines/Makefile -run-make/longjmp-across-rust/Makefile run-make/lto-linkage-used-attr/Makefile run-make/lto-no-link-whole-rlib/Makefile run-make/lto-smoke-c/Makefile diff --git a/tests/run-make/longjmp-across-rust/Makefile b/tests/run-make/longjmp-across-rust/Makefile deleted file mode 100644 index 5fd2d4f855ffe..0000000000000 --- a/tests/run-make/longjmp-across-rust/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,foo) - $(RUSTC) main.rs - $(call RUN,main) diff --git a/tests/run-make/longjmp-across-rust/rmake.rs b/tests/run-make/longjmp-across-rust/rmake.rs new file mode 100644 index 0000000000000..90a527077d21d --- /dev/null +++ b/tests/run-make/longjmp-across-rust/rmake.rs @@ -0,0 +1,18 @@ +// longjmp, an error handling function used in C, is useful +// for jumping out of nested call chains... but it used to accidentally +// trigger Rust's cleanup system in a way that caused an unexpected abortion +// of the program. After this was fixed in #48572, this test compiles and executes +// a program that jumps between Rust and its C library, with longjmp included. For +// the test to succeed, no unexpected abortion should occur. +// See https://github.com/rust-lang/rust/pull/48572 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("foo"); + rustc().input("main.rs").run(); + run("main"); +} From 205bfe72130fc9ea3eb646a5d3d59d0525b8d708 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Mon, 8 Jul 2024 14:35:30 -0400 Subject: [PATCH 06/20] rewrite static-extern-type to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/static-extern-type/Makefile | 6 ------ tests/run-make/static-extern-type/rmake.rs | 16 ++++++++++++++++ 3 files changed, 16 insertions(+), 7 deletions(-) delete mode 100644 tests/run-make/static-extern-type/Makefile create mode 100644 tests/run-make/static-extern-type/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 2f2af6b8bc1dc..51614f84ec360 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -108,7 +108,6 @@ run-make/simd-ffi/Makefile run-make/split-debuginfo/Makefile run-make/stable-symbol-names/Makefile run-make/static-dylib-by-default/Makefile -run-make/static-extern-type/Makefile run-make/staticlib-blank-lib/Makefile run-make/staticlib-dylib-linkage/Makefile run-make/symbol-mangling-hashed/Makefile diff --git a/tests/run-make/static-extern-type/Makefile b/tests/run-make/static-extern-type/Makefile deleted file mode 100644 index 778971543221e..0000000000000 --- a/tests/run-make/static-extern-type/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,define-foo) - $(RUSTC) -ldefine-foo use-foo.rs - $(call RUN,use-foo) || exit 1 diff --git a/tests/run-make/static-extern-type/rmake.rs b/tests/run-make/static-extern-type/rmake.rs new file mode 100644 index 0000000000000..d30153f9c6863 --- /dev/null +++ b/tests/run-make/static-extern-type/rmake.rs @@ -0,0 +1,16 @@ +// Static variables coming from a C library through foreign function interface (FFI) are unsized +// at compile time - and assuming they are sized used to cause an internal compiler error (ICE). +// After this was fixed in #58192, this test checks that external statics can be safely used in +// a program that both compiles and executes successfully. +// See https://github.com/rust-lang/rust/issues/57876 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("define-foo"); + rustc().arg("-ldefine-foo").input("use-foo.rs").run(); + run("use-foo"); +} From bde91785dc4de27787ac20074f3fda70ec47c88d Mon Sep 17 00:00:00 2001 From: Oneirical Date: Mon, 8 Jul 2024 14:51:01 -0400 Subject: [PATCH 07/20] rewrite extern-fn-explicit-align to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../run-make/extern-fn-explicit-align/Makefile | 6 ------ .../run-make/extern-fn-explicit-align/rmake.rs | 17 +++++++++++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) delete mode 100644 tests/run-make/extern-fn-explicit-align/Makefile create mode 100644 tests/run-make/extern-fn-explicit-align/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 51614f84ec360..fde065b929b94 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -24,7 +24,6 @@ run-make/emit-to-stdout/Makefile run-make/export-executable-symbols/Makefile run-make/extern-diff-internal-name/Makefile run-make/extern-flag-disambiguates/Makefile -run-make/extern-fn-explicit-align/Makefile run-make/extern-fn-generic/Makefile run-make/extern-fn-mangle/Makefile run-make/extern-fn-reachable/Makefile diff --git a/tests/run-make/extern-fn-explicit-align/Makefile b/tests/run-make/extern-fn-explicit-align/Makefile deleted file mode 100644 index 3cbbf3839969f..0000000000000 --- a/tests/run-make/extern-fn-explicit-align/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,test) - $(RUSTC) test.rs - $(call RUN,test) || exit 1 diff --git a/tests/run-make/extern-fn-explicit-align/rmake.rs b/tests/run-make/extern-fn-explicit-align/rmake.rs new file mode 100644 index 0000000000000..fb153f92103b2 --- /dev/null +++ b/tests/run-make/extern-fn-explicit-align/rmake.rs @@ -0,0 +1,17 @@ +// The compiler's rules of alignment for indirectly passed values in a 16-byte aligned argument, +// in a C external function, used to be arbitrary. Unexpected behavior would occasionally occur +// and cause memory corruption. This was fixed in #112157, streamlining the way alignment occurs, +// and this test reproduces the case featured in the issue, checking that it compiles and executes +// successfully. +// See https://github.com/rust-lang/rust/issues/80127 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("test"); + rustc().input("test.rs").run(); + run("test"); +} From f7d67d6b6855d0f3880668f2f2d0dabc02caef31 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Mon, 8 Jul 2024 14:57:07 -0400 Subject: [PATCH 08/20] rewrite extern-fn-with-packed-struct to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../extern-fn-with-packed-struct/Makefile | 6 ------ .../extern-fn-with-packed-struct/rmake.rs | 17 +++++++++++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) delete mode 100644 tests/run-make/extern-fn-with-packed-struct/Makefile create mode 100644 tests/run-make/extern-fn-with-packed-struct/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index fde065b929b94..1c6fc57bd818c 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -27,7 +27,6 @@ run-make/extern-flag-disambiguates/Makefile run-make/extern-fn-generic/Makefile run-make/extern-fn-mangle/Makefile run-make/extern-fn-reachable/Makefile -run-make/extern-fn-with-packed-struct/Makefile run-make/extern-fn-with-union/Makefile run-make/extern-multiple-copies/Makefile run-make/extern-multiple-copies2/Makefile diff --git a/tests/run-make/extern-fn-with-packed-struct/Makefile b/tests/run-make/extern-fn-with-packed-struct/Makefile deleted file mode 100644 index 3cbbf3839969f..0000000000000 --- a/tests/run-make/extern-fn-with-packed-struct/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,test) - $(RUSTC) test.rs - $(call RUN,test) || exit 1 diff --git a/tests/run-make/extern-fn-with-packed-struct/rmake.rs b/tests/run-make/extern-fn-with-packed-struct/rmake.rs new file mode 100644 index 0000000000000..e6d8cecd24a2b --- /dev/null +++ b/tests/run-make/extern-fn-with-packed-struct/rmake.rs @@ -0,0 +1,17 @@ +// Packed structs, in C, occupy less bytes in memory, but are more +// vulnerable to alignment errors. Passing them around in a Rust-C foreign +// function interface (FFI) would cause unexpected behavior, until this was +// fixed in #16584. This test checks that a Rust program with a C library +// compiles and executes successfully, even with usage of a packed struct. +// See https://github.com/rust-lang/rust/issues/16574 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("test"); + rustc().input("test.rs").run(); + run("test"); +} From c68d25b0977b5415fdffb48c381ee1c49fb31322 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Mon, 8 Jul 2024 15:06:16 -0400 Subject: [PATCH 09/20] rewrite extern-fn-mangle to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/extern-fn-mangle/Makefile | 6 ------ tests/run-make/extern-fn-mangle/rmake.rs | 16 ++++++++++++++++ 3 files changed, 16 insertions(+), 7 deletions(-) delete mode 100644 tests/run-make/extern-fn-mangle/Makefile create mode 100644 tests/run-make/extern-fn-mangle/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 1c6fc57bd818c..2c272706e7dfc 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -25,7 +25,6 @@ run-make/export-executable-symbols/Makefile run-make/extern-diff-internal-name/Makefile run-make/extern-flag-disambiguates/Makefile run-make/extern-fn-generic/Makefile -run-make/extern-fn-mangle/Makefile run-make/extern-fn-reachable/Makefile run-make/extern-fn-with-union/Makefile run-make/extern-multiple-copies/Makefile diff --git a/tests/run-make/extern-fn-mangle/Makefile b/tests/run-make/extern-fn-mangle/Makefile deleted file mode 100644 index 3cbbf3839969f..0000000000000 --- a/tests/run-make/extern-fn-mangle/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,test) - $(RUSTC) test.rs - $(call RUN,test) || exit 1 diff --git a/tests/run-make/extern-fn-mangle/rmake.rs b/tests/run-make/extern-fn-mangle/rmake.rs new file mode 100644 index 0000000000000..3db8b2a0db0eb --- /dev/null +++ b/tests/run-make/extern-fn-mangle/rmake.rs @@ -0,0 +1,16 @@ +// In this test, the functions foo() and bar() must avoid being mangled, as +// the external C function depends on them to return the correct sum of 3 + 5 = 8. +// This test therefore checks that the compiled and executed program respects the +// #[no_mangle] flags successfully. +// See https://github.com/rust-lang/rust/pull/15831 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("test"); + rustc().input("test.rs").run(); + run("test"); +} From d83ada35ed22598398ee1cc629b8f544c627197f Mon Sep 17 00:00:00 2001 From: Oneirical Date: Tue, 16 Jul 2024 13:45:56 -0400 Subject: [PATCH 10/20] rewrite and rename issue-85401-static-mir --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../bar.rs | 0 .../baz.rs | 0 .../foo.rs | 0 tests/run-make/ice-static-mir/rmake.rs | 42 +++++++++++++++++++ .../run-make/issue-85401-static-mir/Makefile | 16 ------- 6 files changed, 42 insertions(+), 17 deletions(-) rename tests/run-make/{issue-85401-static-mir => ice-static-mir}/bar.rs (100%) rename tests/run-make/{issue-85401-static-mir => ice-static-mir}/baz.rs (100%) rename tests/run-make/{issue-85401-static-mir => ice-static-mir}/foo.rs (100%) create mode 100644 tests/run-make/ice-static-mir/rmake.rs delete mode 100644 tests/run-make/issue-85401-static-mir/Makefile diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 2e26f9344b899..9e9d70bc6c630 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -54,7 +54,6 @@ run-make/issue-36710/Makefile run-make/issue-47551/Makefile run-make/issue-69368/Makefile run-make/issue-84395-lto-embed-bitcode/Makefile -run-make/issue-85401-static-mir/Makefile run-make/issue-88756-default-output/Makefile run-make/issue-97463-abi-param-passing/Makefile run-make/jobserver-error/Makefile diff --git a/tests/run-make/issue-85401-static-mir/bar.rs b/tests/run-make/ice-static-mir/bar.rs similarity index 100% rename from tests/run-make/issue-85401-static-mir/bar.rs rename to tests/run-make/ice-static-mir/bar.rs diff --git a/tests/run-make/issue-85401-static-mir/baz.rs b/tests/run-make/ice-static-mir/baz.rs similarity index 100% rename from tests/run-make/issue-85401-static-mir/baz.rs rename to tests/run-make/ice-static-mir/baz.rs diff --git a/tests/run-make/issue-85401-static-mir/foo.rs b/tests/run-make/ice-static-mir/foo.rs similarity index 100% rename from tests/run-make/issue-85401-static-mir/foo.rs rename to tests/run-make/ice-static-mir/foo.rs diff --git a/tests/run-make/ice-static-mir/rmake.rs b/tests/run-make/ice-static-mir/rmake.rs new file mode 100644 index 0000000000000..2d4ffa379b62b --- /dev/null +++ b/tests/run-make/ice-static-mir/rmake.rs @@ -0,0 +1,42 @@ +// Trying to access mid-level internal representation (MIR) in statics +// used to cause an internal compiler error (ICE), now handled as a proper +// error since #100211. This test checks that the correct error is printed +// during the linking process, and not the ICE. +// See https://github.com/rust-lang/rust/issues/85401 + +use run_make_support::{bin_name, rust_lib_name, rustc}; + +fn main() { + rustc() + .crate_type("rlib") + .crate_name("foo") + .arg("-Crelocation-model=pic") + .edition("2018") + .input("foo.rs") + .arg("-Zalways-encode-mir=yes") + .emit("metadata") + .output("libfoo.rmeta") + .run(); + rustc() + .crate_type("rlib") + .crate_name("bar") + .arg("-Crelocation-model=pic") + .edition("2018") + .input("bar.rs") + .output(rust_lib_name("bar")) + .extern_("foo", "libfoo.rmeta") + .run(); + rustc() + .crate_type("bin") + .crate_name("baz") + .arg("-Crelocation-model=pic") + .edition("2018") + .input("baz.rs") + .output(bin_name("baz")) + .extern_("bar", rust_lib_name("bar")) + .run_fail() + .assert_stderr_contains( + "crate `foo` required to be available in rlib format, but was not found in this form", + ) + .assert_stdout_not_contains("internal compiler error"); +} diff --git a/tests/run-make/issue-85401-static-mir/Makefile b/tests/run-make/issue-85401-static-mir/Makefile deleted file mode 100644 index 47a36b6e45355..0000000000000 --- a/tests/run-make/issue-85401-static-mir/Makefile +++ /dev/null @@ -1,16 +0,0 @@ -include ../tools.mk - -# Regression test for issue #85401 -# Verify that we do not ICE when trying to access MIR for statics, -# but emit an error when linking. - -OUTPUT_FILE := $(TMPDIR)/build-output - -all: - $(RUSTC) --crate-type rlib --crate-name foo -Crelocation-model=pic --edition=2018 foo.rs -Zalways-encode-mir=yes --emit metadata -o $(TMPDIR)/libfoo.rmeta - $(RUSTC) --crate-type rlib --crate-name bar -Crelocation-model=pic --edition=2018 bar.rs -o $(TMPDIR)/libbar.rlib --extern=foo=$(TMPDIR)/libfoo.rmeta - $(RUSTC) --crate-type bin --crate-name baz -Crelocation-model=pic --edition=2018 baz.rs -o $(TMPDIR)/baz -L $(TMPDIR) --extern=bar=$(TMPDIR)/libbar.rlib > $(OUTPUT_FILE) 2>&1; [ $$? -eq 1 ] - cat $(OUTPUT_FILE) - $(CGREP) 'crate `foo` required to be available in rlib format, but was not found in this form' < $(OUTPUT_FILE) - # -v tests are fragile, hopefully this text won't change - $(CGREP) -v "internal compiler error" < $(OUTPUT_FILE) From 890ef1180b96683602ecd7ce0fe19a431d6474e2 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Tue, 16 Jul 2024 14:01:02 -0400 Subject: [PATCH 11/20] rewrite missing-crate-dependency to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../run-make/missing-crate-dependency/Makefile | 9 --------- .../run-make/missing-crate-dependency/rmake.rs | 17 +++++++++++++++++ 3 files changed, 17 insertions(+), 10 deletions(-) delete mode 100644 tests/run-make/missing-crate-dependency/Makefile create mode 100644 tests/run-make/missing-crate-dependency/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 9e9d70bc6c630..349f0e3169f41 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -75,7 +75,6 @@ run-make/macos-deployment-target/Makefile run-make/macos-fat-archive/Makefile run-make/manual-link/Makefile run-make/min-global-align/Makefile -run-make/missing-crate-dependency/Makefile run-make/native-link-modifier-bundle/Makefile run-make/native-link-modifier-whole-archive/Makefile run-make/no-alloc-shim/Makefile diff --git a/tests/run-make/missing-crate-dependency/Makefile b/tests/run-make/missing-crate-dependency/Makefile deleted file mode 100644 index 7c271ab8a90de..0000000000000 --- a/tests/run-make/missing-crate-dependency/Makefile +++ /dev/null @@ -1,9 +0,0 @@ -include ../tools.mk - -all: - $(RUSTC) --crate-type=rlib crateA.rs - $(RUSTC) --crate-type=rlib crateB.rs - $(call REMOVE_RLIBS,crateA) - # Ensure crateC fails to compile since dependency crateA is missing - $(RUSTC) crateC.rs 2>&1 | \ - $(CGREP) 'can'"'"'t find crate for `crateA` which `crateB` depends on' diff --git a/tests/run-make/missing-crate-dependency/rmake.rs b/tests/run-make/missing-crate-dependency/rmake.rs new file mode 100644 index 0000000000000..ab916fc6b6131 --- /dev/null +++ b/tests/run-make/missing-crate-dependency/rmake.rs @@ -0,0 +1,17 @@ +// A simple smoke test to check that rustc fails compilation +// and outputs a helpful message when a dependency is missing +// in a dependency chain. +// See https://github.com/rust-lang/rust/issues/12146 + +use run_make_support::{fs_wrapper, rust_lib_name, rustc}; + +fn main() { + rustc().crate_type("rlib").input("crateA.rs").run(); + rustc().crate_type("rlib").input("crateB.rs").run(); + fs_wrapper::remove_file(rust_lib_name("crateA")); + // Ensure that crateC fails to compile, as the crateA dependency is missing. + rustc() + .input("crateC.rs") + .run_fail() + .assert_stderr_contains("can't find crate for `crateA` which `crateB` depends on"); +} From 3ba62f0a63c6b0092b6fec3bbc8877045073d827 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Tue, 16 Jul 2024 14:12:12 -0400 Subject: [PATCH 12/20] rewrite unstable-flag-required to rmake --- src/tools/tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/missing-crate-dependency/rmake.rs | 4 ++-- tests/run-make/unstable-flag-required/Makefile | 4 ---- tests/run-make/unstable-flag-required/rmake.rs | 12 ++++++++++++ 4 files changed, 14 insertions(+), 7 deletions(-) delete mode 100644 tests/run-make/unstable-flag-required/Makefile create mode 100644 tests/run-make/unstable-flag-required/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 349f0e3169f41..282b615c5bf4d 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -120,5 +120,4 @@ run-make/test-benches/Makefile run-make/thumb-none-cortex-m/Makefile run-make/thumb-none-qemu/Makefile run-make/translation/Makefile -run-make/unstable-flag-required/Makefile run-make/x86_64-fortanix-unknown-sgx-lvi/Makefile diff --git a/tests/run-make/missing-crate-dependency/rmake.rs b/tests/run-make/missing-crate-dependency/rmake.rs index ab916fc6b6131..dae77032f7d4a 100644 --- a/tests/run-make/missing-crate-dependency/rmake.rs +++ b/tests/run-make/missing-crate-dependency/rmake.rs @@ -3,12 +3,12 @@ // in a dependency chain. // See https://github.com/rust-lang/rust/issues/12146 -use run_make_support::{fs_wrapper, rust_lib_name, rustc}; +use run_make_support::{rfs, rust_lib_name, rustc}; fn main() { rustc().crate_type("rlib").input("crateA.rs").run(); rustc().crate_type("rlib").input("crateB.rs").run(); - fs_wrapper::remove_file(rust_lib_name("crateA")); + rfs::remove_file(rust_lib_name("crateA")); // Ensure that crateC fails to compile, as the crateA dependency is missing. rustc() .input("crateC.rs") diff --git a/tests/run-make/unstable-flag-required/Makefile b/tests/run-make/unstable-flag-required/Makefile deleted file mode 100644 index 17dd15b079c62..0000000000000 --- a/tests/run-make/unstable-flag-required/Makefile +++ /dev/null @@ -1,4 +0,0 @@ -include ../tools.mk - -all: - $(RUSTDOC) --output-format=json x.html 2>&1 | diff - output-format-json.stderr diff --git a/tests/run-make/unstable-flag-required/rmake.rs b/tests/run-make/unstable-flag-required/rmake.rs new file mode 100644 index 0000000000000..c521436c203d5 --- /dev/null +++ b/tests/run-make/unstable-flag-required/rmake.rs @@ -0,0 +1,12 @@ +// The flag `--output-format` is unauthorized on beta and stable releases, which led +// to confusion for maintainers doing testing on nightly. Tying it to an unstable flag +// elucidates this, and this test checks that `--output-format` cannot be passed on its +// own. +// See https://github.com/rust-lang/rust/pull/82497 + +use run_make_support::{diff, rustdoc}; + +fn main() { + let out = rustdoc().output_format("json").input("x.html").run_fail().stderr_utf8(); + diff().expected_file("output-format-json.stderr").actual_text("actual-json", out).run(); +} From 7f3f34db740481ad814889e9393aaf51168b36c3 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 18 Jul 2024 06:32:53 +0000 Subject: [PATCH 13/20] Mark myself as on leave --- triagebot.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/triagebot.toml b/triagebot.toml index 2f36eb9d1c90b..5335cf4e4bf51 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -905,7 +905,7 @@ cc = ["@kobzol"] [assign] warn_non_default_branch = true contributing_url = "https://rustc-dev-guide.rust-lang.org/getting-started.html" -users_on_vacation = ["jyn514", "jhpratt"] +users_on_vacation = ["jyn514", "jhpratt", "oli-obk"] [assign.adhoc_groups] compiler-team = [ From 487802d6c8a74ee375d2f71e3ff97cea11cc9c18 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 17 Jul 2024 00:12:24 +1000 Subject: [PATCH 14/20] Remove `TrailingToken`. It's used in `Parser::collect_tokens_trailing_token` to decide whether to capture a trailing token. But the callers actually know whether to capture a trailing token, so it's simpler for them to just pass in a bool. Also, the `TrailingToken::Gt` case was weird, because it didn't result in a trailing token being captured. It could have been subsumed by the `TrailingToken::MaybeComma` case, and it effectively is in the new code. --- .../rustc_parse/src/parser/attr_wrapper.rs | 32 ++++++------------- compiler/rustc_parse/src/parser/expr.rs | 26 ++++++--------- compiler/rustc_parse/src/parser/generics.rs | 12 +++---- compiler/rustc_parse/src/parser/item.rs | 25 +++++++-------- compiler/rustc_parse/src/parser/mod.rs | 12 +------ compiler/rustc_parse/src/parser/pat.rs | 7 ++-- compiler/rustc_parse/src/parser/stmt.rs | 20 +++--------- 7 files changed, 45 insertions(+), 89 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 1123c31f55135..7e56e92f87b31 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -1,5 +1,5 @@ -use super::{Capturing, FlatToken, ForceCollect, Parser, ReplaceRange, TokenCursor, TrailingToken}; -use rustc_ast::token::{self, Delimiter, Token, TokenKind}; +use super::{Capturing, FlatToken, ForceCollect, Parser, ReplaceRange, TokenCursor}; +use rustc_ast::token::{Delimiter, Token, TokenKind}; use rustc_ast::tokenstream::{AttrTokenStream, AttrTokenTree, AttrsTarget, DelimSpacing}; use rustc_ast::tokenstream::{DelimSpan, LazyAttrTokenStream, Spacing, ToAttrTokenStream}; use rustc_ast::{self as ast}; @@ -165,8 +165,10 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl { impl<'a> Parser<'a> { /// Records all tokens consumed by the provided callback, /// including the current token. These tokens are collected - /// into a `LazyAttrTokenStream`, and returned along with the result - /// of the callback. + /// into a `LazyAttrTokenStream`, and returned along with the first part of + /// the callback's result. The second (bool) part of the callback's result + /// indicates if an extra token should be captured, e.g. a comma or + /// semicolon. /// /// The `attrs` passed in are in `AttrWrapper` form, which is opaque. The /// `AttrVec` within is passed to `f`. See the comment on `AttrWrapper` for @@ -187,7 +189,7 @@ impl<'a> Parser<'a> { &mut self, attrs: AttrWrapper, force_collect: ForceCollect, - f: impl FnOnce(&mut Self, ast::AttrVec) -> PResult<'a, (R, TrailingToken)>, + f: impl FnOnce(&mut Self, ast::AttrVec) -> PResult<'a, (R, bool)>, ) -> PResult<'a, R> { // We only bail out when nothing could possibly observe the collected tokens: // 1. We cannot be force collecting tokens (since force-collecting requires tokens @@ -212,7 +214,7 @@ impl<'a> Parser<'a> { let has_outer_attrs = !attrs.attrs.is_empty(); let replace_ranges_start = self.capture_state.replace_ranges.len(); - let (mut ret, trailing) = { + let (mut ret, capture_trailing) = { let prev_capturing = mem::replace(&mut self.capture_state.capturing, Capturing::Yes); let ret_and_trailing = f(self, attrs.attrs); self.capture_state.capturing = prev_capturing; @@ -266,27 +268,13 @@ impl<'a> Parser<'a> { let replace_ranges_end = self.capture_state.replace_ranges.len(); - // Capture a trailing token if requested by the callback 'f' - let captured_trailing = match trailing { - TrailingToken::None => false, - TrailingToken::Gt => { - assert_eq!(self.token.kind, token::Gt); - false - } - TrailingToken::Semi => { - assert_eq!(self.token.kind, token::Semi); - true - } - TrailingToken::MaybeComma => self.token.kind == token::Comma, - }; - assert!( - !(self.break_last_token && captured_trailing), + !(self.break_last_token && capture_trailing), "Cannot set break_last_token and have trailing token" ); let end_pos = self.num_bump_calls - + captured_trailing as u32 + + capture_trailing as u32 // If we 'broke' the last token (e.g. breaking a '>>' token to two '>' tokens), then // extend the range of captured tokens to include it, since the parser was not actually // bumped past it. When the `LazyAttrTokenStream` gets converted into an diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 0e7497cea4133..2542108728f7d 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -5,7 +5,7 @@ use super::pat::{CommaRecoveryMode, Expected, RecoverColon, RecoverComma}; use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign}; use super::{ AttrWrapper, BlockMode, ClosureSpans, ForceCollect, Parser, PathStyle, Restrictions, - SemiColonMode, SeqSep, TokenType, Trailing, TrailingToken, + SemiColonMode, SeqSep, TokenType, Trailing, }; use crate::errors; @@ -2474,7 +2474,7 @@ impl<'a> Parser<'a> { id: DUMMY_NODE_ID, is_placeholder: false, }, - TrailingToken::MaybeComma, + this.token == token::Comma, )) }) } @@ -3257,7 +3257,7 @@ impl<'a> Parser<'a> { id: DUMMY_NODE_ID, is_placeholder: false, }, - TrailingToken::None, + false, )) }) } @@ -3766,7 +3766,7 @@ impl<'a> Parser<'a> { id: DUMMY_NODE_ID, is_placeholder: false, }, - TrailingToken::MaybeComma, + this.token == token::Comma, )) }) } @@ -3862,18 +3862,12 @@ impl<'a> Parser<'a> { ) -> PResult<'a, P> { self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| { let res = f(this, attrs)?; - let trailing = if this.restrictions.contains(Restrictions::STMT_EXPR) - && this.token.kind == token::Semi - { - TrailingToken::Semi - } else if this.token.kind == token::Gt { - TrailingToken::Gt - } else { - // FIXME - pass this through from the place where we know - // we need a comma, rather than assuming that `#[attr] expr,` - // always captures a trailing comma - TrailingToken::MaybeComma - }; + let trailing = (this.restrictions.contains(Restrictions::STMT_EXPR) + && this.token.kind == token::Semi) + // FIXME: pass an additional condition through from the place + // where we know we need a comma, rather than assuming that + // `#[attr] expr,` always captures a trailing comma. + || this.token.kind == token::Comma; Ok((res, trailing)) }) } diff --git a/compiler/rustc_parse/src/parser/generics.rs b/compiler/rustc_parse/src/parser/generics.rs index 10c7715c7dcd5..523538e9643ac 100644 --- a/compiler/rustc_parse/src/parser/generics.rs +++ b/compiler/rustc_parse/src/parser/generics.rs @@ -4,7 +4,7 @@ use crate::errors::{ WhereClauseBeforeTupleStructBodySugg, }; -use super::{ForceCollect, Parser, TrailingToken}; +use super::{ForceCollect, Parser}; use ast::token::Delimiter; use rustc_ast::token; @@ -229,13 +229,13 @@ impl<'a> Parser<'a> { span: where_predicate.span(), }); // FIXME - try to continue parsing other generics? - return Ok((None, TrailingToken::None)); + return Ok((None, false)); } Err(err) => { err.cancel(); // FIXME - maybe we should overwrite 'self' outside of `collect_tokens`? this.restore_snapshot(snapshot); - return Ok((None, TrailingToken::None)); + return Ok((None, false)); } } } else { @@ -249,14 +249,14 @@ impl<'a> Parser<'a> { .emit_err(errors::AttrWithoutGenerics { span: attrs[0].span }); } } - return Ok((None, TrailingToken::None)); + return Ok((None, false)); }; if !this.eat(&token::Comma) { done = true; } - // We just ate the comma, so no need to use `TrailingToken` - Ok((param, TrailingToken::None)) + // We just ate the comma, so no need to capture the trailing token. + Ok((param, false)) })?; if let Some(param) = param { diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index b964e8aa665d7..78722ba26cbd1 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -1,8 +1,6 @@ use super::diagnostics::{dummy_arg, ConsumeClosingDelim}; use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign}; -use super::{ - AttrWrapper, FollowedByType, ForceCollect, Parser, PathStyle, Trailing, TrailingToken, -}; +use super::{AttrWrapper, FollowedByType, ForceCollect, Parser, PathStyle, Trailing}; use crate::errors::{self, MacroExpandsToAdtField}; use crate::fluent_generated as fluent; use crate::maybe_whole; @@ -146,7 +144,7 @@ impl<'a> Parser<'a> { let span = lo.to(this.prev_token.span); let id = DUMMY_NODE_ID; let item = Item { ident, attrs, id, kind, vis, span, tokens: None }; - return Ok((Some(item), TrailingToken::None)); + return Ok((Some(item), false)); } // At this point, we have failed to parse an item. @@ -161,7 +159,7 @@ impl<'a> Parser<'a> { if !attrs_allowed { this.recover_attrs_no_item(&attrs)?; } - Ok((None, TrailingToken::None)) + Ok((None, false)) }) } @@ -1555,7 +1553,7 @@ impl<'a> Parser<'a> { let vis = this.parse_visibility(FollowedByType::No)?; if !this.recover_nested_adt_item(kw::Enum)? { - return Ok((None, TrailingToken::None)); + return Ok((None, false)); } let ident = this.parse_field_ident("enum", vlo)?; @@ -1567,7 +1565,7 @@ impl<'a> Parser<'a> { this.bump(); this.parse_delim_args()?; - return Ok((None, TrailingToken::MaybeComma)); + return Ok((None, this.token == token::Comma)); } let struct_def = if this.check(&token::OpenDelim(Delimiter::Brace)) { @@ -1624,7 +1622,7 @@ impl<'a> Parser<'a> { is_placeholder: false, }; - Ok((Some(vr), TrailingToken::MaybeComma)) + Ok((Some(vr), this.token == token::Comma)) }, ) .map_err(|mut err| { @@ -1816,7 +1814,7 @@ impl<'a> Parser<'a> { attrs, is_placeholder: false, }, - TrailingToken::MaybeComma, + p.token == token::Comma, )) }) }) @@ -1831,8 +1829,7 @@ impl<'a> Parser<'a> { self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| { let lo = this.token.span; let vis = this.parse_visibility(FollowedByType::No)?; - this.parse_single_struct_field(adt_ty, lo, vis, attrs) - .map(|field| (field, TrailingToken::None)) + this.parse_single_struct_field(adt_ty, lo, vis, attrs).map(|field| (field, false)) }) } @@ -2735,7 +2732,7 @@ impl<'a> Parser<'a> { if let Some(mut param) = this.parse_self_param()? { param.attrs = attrs; let res = if first_param { Ok(param) } else { this.recover_bad_self_param(param) }; - return Ok((res?, TrailingToken::None)); + return Ok((res?, false)); } let is_name_required = match this.token.kind { @@ -2751,7 +2748,7 @@ impl<'a> Parser<'a> { this.parameter_without_type(&mut err, pat, is_name_required, first_param) { let guar = err.emit(); - Ok((dummy_arg(ident, guar), TrailingToken::None)) + Ok((dummy_arg(ident, guar), false)) } else { Err(err) }; @@ -2794,7 +2791,7 @@ impl<'a> Parser<'a> { Ok(( Param { attrs, id: ast::DUMMY_NODE_ID, is_placeholder: false, pat, span, ty }, - TrailingToken::None, + false, )) }) } diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 0117f993bcb33..c03527acb2cfe 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -91,16 +91,6 @@ pub enum ForceCollect { No, } -#[derive(Debug, Eq, PartialEq)] -pub enum TrailingToken { - None, - Semi, - Gt, - /// If the trailing token is a comma, then capture it - /// Otherwise, ignore the trailing token - MaybeComma, -} - #[macro_export] macro_rules! maybe_whole { ($p:expr, $constructor:ident, |$x:ident| $e:expr) => { @@ -1508,7 +1498,7 @@ impl<'a> Parser<'a> { self.collect_tokens_trailing_token( AttrWrapper::empty(), ForceCollect::Yes, - |this, _attrs| Ok((f(this)?, TrailingToken::None)), + |this, _attrs| Ok((f(this)?, false)), ) } diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index 245b730bbe450..18d531faeaa15 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -1,4 +1,4 @@ -use super::{ForceCollect, Parser, PathStyle, Restrictions, Trailing, TrailingToken}; +use super::{ForceCollect, Parser, PathStyle, Restrictions, Trailing}; use crate::errors::{ self, AmbiguousRangePattern, DotDotDotForRemainingFields, DotDotDotRangeToPatternNotAllowed, DotDotDotRestPattern, EnumPatternInsteadOfIdentifier, ExpectedBindingLeftOfAt, @@ -1315,9 +1315,8 @@ impl<'a> Parser<'a> { last_non_comma_dotdot_span = Some(this.prev_token.span); - // We just ate a comma, so there's no need to use - // `TrailingToken::Comma` - Ok((field, TrailingToken::None)) + // We just ate a comma, so there's no need to capture a trailing token. + Ok((field, false)) })?; fields.push(field) diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index e7fcbf9c20fa9..3ec891b4eea34 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -3,7 +3,6 @@ use super::diagnostics::AttemptLocalParseRecovery; use super::expr::LhsExpr; use super::pat::{PatternLocation, RecoverComma}; use super::path::PathStyle; -use super::TrailingToken; use super::{ AttrWrapper, BlockMode, FnParseMode, ForceCollect, Parser, Restrictions, SemiColonMode, }; @@ -149,11 +148,7 @@ impl<'a> Parser<'a> { if this.eat(&token::Not) { let stmt_mac = this.parse_stmt_mac(lo, attrs, path)?; - if this.token == token::Semi { - return Ok((stmt_mac, TrailingToken::Semi)); - } else { - return Ok((stmt_mac, TrailingToken::None)); - } + return Ok((stmt_mac, this.token == token::Semi)); } let expr = if this.eat(&token::OpenDelim(Delimiter::Brace)) { @@ -167,7 +162,7 @@ impl<'a> Parser<'a> { this.parse_expr_dot_or_call_with(attrs, expr, lo) })?; // `DUMMY_SP` will get overwritten later in this function - Ok((this.mk_stmt(rustc_span::DUMMY_SP, StmtKind::Expr(expr)), TrailingToken::None)) + Ok((this.mk_stmt(rustc_span::DUMMY_SP, StmtKind::Expr(expr)), false)) })?; if let StmtKind::Expr(expr) = stmt.kind { @@ -241,10 +236,7 @@ impl<'a> Parser<'a> { self.collect_tokens_trailing_token(attrs, ForceCollect::Yes, |this, attrs| { let local = this.parse_local(attrs)?; // FIXME - maybe capture semicolon in recovery? - Ok(( - this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Let(local)), - TrailingToken::None, - )) + Ok((this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Let(local)), false)) })?; self.dcx() .emit_err(errors::InvalidVariableDeclaration { span: lo, sub: subdiagnostic(lo) }); @@ -261,11 +253,7 @@ impl<'a> Parser<'a> { self.collect_tokens_trailing_token(attrs, force_collect, |this, attrs| { this.expect_keyword(kw::Let)?; let local = this.parse_local(attrs)?; - let trailing = if capture_semi && this.token.kind == token::Semi { - TrailingToken::Semi - } else { - TrailingToken::None - }; + let trailing = capture_semi && this.token.kind == token::Semi; Ok((this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Let(local)), trailing)) }) } From 69157bde3b309f99c4591fbfc93662ad75a611a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 18 Jul 2024 08:42:45 +0200 Subject: [PATCH 15/20] Add missing GHA group for building `llvm-bitcode-linker` --- src/bootstrap/src/core/build_steps/tool.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 7bc410b9e8878..2f8b41334fc7c 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -858,6 +858,15 @@ impl Step for LlvmBitcodeLinker { &self.extra_features, ); + let _guard = builder.msg_tool( + Kind::Build, + Mode::ToolRustc, + bin_name, + self.compiler.stage, + &self.compiler.host, + &self.target, + ); + cargo.into_cmd().run(builder); let tool_out = builder From fa74a9e6aa525a285cfd530cdea2ddeb9fca013c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 13 Jul 2024 16:13:55 +0200 Subject: [PATCH 16/20] valtree construction: keep track of which type was valtree-incompatible --- compiler/rustc_codegen_ssa/src/mir/constant.rs | 5 +++-- compiler/rustc_const_eval/src/const_eval/mod.rs | 10 +++++----- .../rustc_const_eval/src/const_eval/valtrees.rs | 16 ++++++++-------- compiler/rustc_infer/src/infer/mod.rs | 6 +++--- compiler/rustc_middle/src/mir/interpret/error.rs | 8 +++++--- compiler/rustc_middle/src/query/erase.rs | 7 ++++--- compiler/rustc_middle/src/ty/consts.rs | 3 +-- compiler/rustc_mir_build/src/thir/pattern/mod.rs | 7 +++---- .../rustc_trait_selection/src/solve/delegate.rs | 4 ++-- .../src/traits/auto_trait.rs | 4 ++-- .../clippy/clippy_lints/src/non_copy_const.rs | 2 +- 11 files changed, 37 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/constant.rs b/compiler/rustc_codegen_ssa/src/mir/constant.rs index 822f5c2c44a2e..35e9a3b7dc206 100644 --- a/compiler/rustc_codegen_ssa/src/mir/constant.rs +++ b/compiler/rustc_codegen_ssa/src/mir/constant.rs @@ -37,13 +37,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { pub fn eval_unevaluated_mir_constant_to_valtree( &self, constant: &mir::ConstOperand<'tcx>, - ) -> Result>, ErrorHandled> { + ) -> Result, Ty<'tcx>>, ErrorHandled> { let uv = match self.monomorphize(constant.const_) { mir::Const::Unevaluated(uv, _) => uv.shrink(), mir::Const::Ty(_, c) => match c.kind() { // A constant that came from a const generic but was then used as an argument to old-style // simd_shuffle (passing as argument instead of as a generic param). - rustc_type_ir::ConstKind::Value(_, valtree) => return Ok(Some(valtree)), + rustc_type_ir::ConstKind::Value(_, valtree) => return Ok(Ok(valtree)), other => span_bug!(constant.span, "{other:#?}"), }, // We should never encounter `Const::Val` unless MIR opts (like const prop) evaluate @@ -70,6 +70,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let val = self .eval_unevaluated_mir_constant_to_valtree(constant) .ok() + .map(|x| x.ok()) .flatten() .map(|val| { let field_ty = ty.builtin_index().unwrap(); diff --git a/compiler/rustc_const_eval/src/const_eval/mod.rs b/compiler/rustc_const_eval/src/const_eval/mod.rs index 4ae4816e33ab9..3a6dc81eff11f 100644 --- a/compiler/rustc_const_eval/src/const_eval/mod.rs +++ b/compiler/rustc_const_eval/src/const_eval/mod.rs @@ -27,15 +27,15 @@ pub(crate) use valtrees::{eval_to_valtree, valtree_to_const_value}; // We forbid type-level constants that contain more than `VALTREE_MAX_NODES` nodes. const VALTREE_MAX_NODES: usize = 100000; -pub(crate) enum ValTreeCreationError { +pub(crate) enum ValTreeCreationError<'tcx> { NodesOverflow, /// Values of this type, or this particular value, are not supported as valtrees. - NonSupportedType, + NonSupportedType(Ty<'tcx>), } -pub(crate) type ValTreeCreationResult<'tcx> = Result, ValTreeCreationError>; +pub(crate) type ValTreeCreationResult<'tcx> = Result, ValTreeCreationError<'tcx>>; -impl From> for ValTreeCreationError { - fn from(err: InterpErrorInfo<'_>) -> Self { +impl<'tcx> From> for ValTreeCreationError<'tcx> { + fn from(err: InterpErrorInfo<'tcx>) -> Self { ty::tls::with(|tcx| { bug!( "Unexpected Undefined Behavior error during valtree construction: {}", diff --git a/compiler/rustc_const_eval/src/const_eval/valtrees.rs b/compiler/rustc_const_eval/src/const_eval/valtrees.rs index 2e8ad445cf5e8..3bc01510730b4 100644 --- a/compiler/rustc_const_eval/src/const_eval/valtrees.rs +++ b/compiler/rustc_const_eval/src/const_eval/valtrees.rs @@ -120,13 +120,13 @@ fn const_to_valtree_inner<'tcx>( // We could allow wide raw pointers where both sides are integers in the future, // but for now we reject them. if matches!(val.layout.abi, Abi::ScalarPair(..)) { - return Err(ValTreeCreationError::NonSupportedType); + return Err(ValTreeCreationError::NonSupportedType(ty)); } let val = val.to_scalar(); // We are in the CTFE machine, so ptr-to-int casts will fail. // This can only be `Ok` if `val` already is an integer. let Ok(val) = val.try_to_scalar_int() else { - return Err(ValTreeCreationError::NonSupportedType); + return Err(ValTreeCreationError::NonSupportedType(ty)); }; // It's just a ScalarInt! Ok(ty::ValTree::Leaf(val)) @@ -134,7 +134,7 @@ fn const_to_valtree_inner<'tcx>( // Technically we could allow function pointers (represented as `ty::Instance`), but this is not guaranteed to // agree with runtime equality tests. - ty::FnPtr(_) => Err(ValTreeCreationError::NonSupportedType), + ty::FnPtr(_) => Err(ValTreeCreationError::NonSupportedType(ty)), ty::Ref(_, _, _) => { let derefd_place = ecx.deref_pointer(place)?; @@ -148,7 +148,7 @@ fn const_to_valtree_inner<'tcx>( // resolving their backing type, even if we can do that at const eval time. We may // hypothetically be able to allow `dyn StructuralPartialEq` trait objects in the future, // but it is unclear if this is useful. - ty::Dynamic(..) => Err(ValTreeCreationError::NonSupportedType), + ty::Dynamic(..) => Err(ValTreeCreationError::NonSupportedType(ty)), ty::Tuple(elem_tys) => { branches(ecx, place, elem_tys.len(), None, num_nodes) @@ -156,7 +156,7 @@ fn const_to_valtree_inner<'tcx>( ty::Adt(def, _) => { if def.is_union() { - return Err(ValTreeCreationError::NonSupportedType); + return Err(ValTreeCreationError::NonSupportedType(ty)); } else if def.variants().is_empty() { bug!("uninhabited types should have errored and never gotten converted to valtree") } @@ -180,7 +180,7 @@ fn const_to_valtree_inner<'tcx>( | ty::Closure(..) | ty::CoroutineClosure(..) | ty::Coroutine(..) - | ty::CoroutineWitness(..) => Err(ValTreeCreationError::NonSupportedType), + | ty::CoroutineWitness(..) => Err(ValTreeCreationError::NonSupportedType(ty)), } } @@ -251,7 +251,7 @@ pub(crate) fn eval_to_valtree<'tcx>( let valtree_result = const_to_valtree_inner(&ecx, &place, &mut num_nodes); match valtree_result { - Ok(valtree) => Ok(Some(valtree)), + Ok(valtree) => Ok(Ok(valtree)), Err(err) => { let did = cid.instance.def_id(); let global_const_id = cid.display(tcx); @@ -262,7 +262,7 @@ pub(crate) fn eval_to_valtree<'tcx>( tcx.dcx().emit_err(MaxNumNodesInConstErr { span, global_const_id }); Err(handled.into()) } - ValTreeCreationError::NonSupportedType => Ok(None), + ValTreeCreationError::NonSupportedType(ty) => Ok(Err(ty)), } } } diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index cfef1f1301564..c9073d8c23e5a 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -1427,17 +1427,17 @@ impl<'tcx> InferCtxt<'tcx> { span: Span, ) -> Result, ErrorHandled> { match self.const_eval_resolve(param_env, unevaluated, span) { - Ok(Some(val)) => Ok(ty::Const::new_value( + Ok(Ok(val)) => Ok(ty::Const::new_value( self.tcx, val, self.tcx.type_of(unevaluated.def).instantiate(self.tcx, unevaluated.args), )), - Ok(None) => { + Ok(Err(bad_ty)) => { let tcx = self.tcx; let def_id = unevaluated.def; span_bug!( tcx.def_span(def_id), - "unable to construct a constant value for the unevaluated constant {:?}", + "unable to construct a valtree for the unevaluated constant {:?}: type {bad_ty} is not valtree-compatible", unevaluated ); } diff --git a/compiler/rustc_middle/src/mir/interpret/error.rs b/compiler/rustc_middle/src/mir/interpret/error.rs index 6a8498abaf933..9df19565ab383 100644 --- a/compiler/rustc_middle/src/mir/interpret/error.rs +++ b/compiler/rustc_middle/src/mir/interpret/error.rs @@ -90,9 +90,11 @@ TrivialTypeTraversalImpls! { ErrorHandled } pub type EvalToAllocationRawResult<'tcx> = Result, ErrorHandled>; pub type EvalStaticInitializerRawResult<'tcx> = Result, ErrorHandled>; pub type EvalToConstValueResult<'tcx> = Result, ErrorHandled>; -/// `Ok(None)` indicates the constant was fine, but the valtree couldn't be constructed. -/// This is needed in `thir::pattern::lower_inline_const`. -pub type EvalToValTreeResult<'tcx> = Result>, ErrorHandled>; +/// `Ok(Err(ty))` indicates the constant was fine, but the valtree couldn't be constructed +/// because the value containts something of type `ty` that is not valtree-compatible. +/// The caller can then show an appropriate error; the query does not have the +/// necssary context to give good user-facing errors for this case. +pub type EvalToValTreeResult<'tcx> = Result, Ty<'tcx>>, ErrorHandled>; #[cfg(target_pointer_width = "64")] rustc_data_structures::static_assert_size!(InterpErrorInfo<'_>, 8); diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs index 301c9911b44cd..d9fa5b02f7f74 100644 --- a/compiler/rustc_middle/src/query/erase.rs +++ b/compiler/rustc_middle/src/query/erase.rs @@ -157,9 +157,10 @@ impl EraseType for Result, mir::interpret::ErrorHandled> { type Result = [u8; size_of::, mir::interpret::ErrorHandled>>()]; } -impl EraseType for Result>, mir::interpret::ErrorHandled> { - type Result = - [u8; size_of::>, mir::interpret::ErrorHandled>>()]; +impl EraseType for Result, Ty<'_>>, mir::interpret::ErrorHandled> { + type Result = [u8; size_of::< + Result, Ty<'static>>, mir::interpret::ErrorHandled>, + >()]; } impl EraseType for Result<&'_ ty::List>, ty::util::AlwaysRequiresDrop> { diff --git a/compiler/rustc_middle/src/ty/consts.rs b/compiler/rustc_middle/src/ty/consts.rs index 32d01d07c17e7..d4c39a25a1f28 100644 --- a/compiler/rustc_middle/src/ty/consts.rs +++ b/compiler/rustc_middle/src/ty/consts.rs @@ -328,8 +328,7 @@ impl<'tcx> Const<'tcx> { let (param_env, unevaluated) = unevaluated.prepare_for_eval(tcx, param_env); // try to resolve e.g. associated constants to their definition on an impl, and then // evaluate the const. - let Some(c) = tcx.const_eval_resolve_for_typeck(param_env, unevaluated, span)? - else { + let Ok(c) = tcx.const_eval_resolve_for_typeck(param_env, unevaluated, span)? else { // This can happen when we run on ill-typed code. let e = tcx.dcx().span_delayed_bug( span, diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index fd778ef78a3ee..25b7d24de3979 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -581,8 +581,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { .tcx .const_eval_global_id_for_typeck(param_env_reveal_all, cid, span) .map(|val| match val { - Some(valtree) => mir::Const::Ty(ty, ty::Const::new_value(self.tcx, valtree, ty)), - None => mir::Const::Val( + Ok(valtree) => mir::Const::Ty(ty, ty::Const::new_value(self.tcx, valtree, ty)), + Err(_) => mir::Const::Val( self.tcx .const_eval_global_id(param_env_reveal_all, cid, span) .expect("const_eval_global_id_for_typeck should have already failed"), @@ -682,8 +682,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { // First try using a valtree in order to destructure the constant into a pattern. // FIXME: replace "try to do a thing, then fall back to another thing" // but something more principled, like a trait query checking whether this can be turned into a valtree. - if let Ok(Some(valtree)) = self.tcx.const_eval_resolve_for_typeck(self.param_env, ct, span) - { + if let Ok(Ok(valtree)) = self.tcx.const_eval_resolve_for_typeck(self.param_env, ct, span) { let subpattern = self.const_to_pat( Const::Ty(ty, ty::Const::new_value(self.tcx, valtree, ty)), id, diff --git a/compiler/rustc_trait_selection/src/solve/delegate.rs b/compiler/rustc_trait_selection/src/solve/delegate.rs index f67518a577e2b..ade26a4092057 100644 --- a/compiler/rustc_trait_selection/src/solve/delegate.rs +++ b/compiler/rustc_trait_selection/src/solve/delegate.rs @@ -87,12 +87,12 @@ impl<'tcx> rustc_next_trait_solver::delegate::SolverDelegate for SolverDelegate< ) -> Option> { use rustc_middle::mir::interpret::ErrorHandled; match self.const_eval_resolve(param_env, unevaluated, DUMMY_SP) { - Ok(Some(val)) => Some(ty::Const::new_value( + Ok(Ok(val)) => Some(ty::Const::new_value( self.tcx, val, self.tcx.type_of(unevaluated.def).instantiate(self.tcx, unevaluated.args), )), - Ok(None) | Err(ErrorHandled::TooGeneric(_)) => None, + Ok(Err(_)) | Err(ErrorHandled::TooGeneric(_)) => None, Err(ErrorHandled::Reported(e, _)) => Some(ty::Const::new_error(self.tcx, e.into())), } } diff --git a/compiler/rustc_trait_selection/src/traits/auto_trait.rs b/compiler/rustc_trait_selection/src/traits/auto_trait.rs index 1d32ef2ccd94b..796f7fd5a54d1 100644 --- a/compiler/rustc_trait_selection/src/traits/auto_trait.rs +++ b/compiler/rustc_trait_selection/src/traits/auto_trait.rs @@ -765,8 +765,8 @@ impl<'tcx> AutoTraitFinder<'tcx> { unevaluated, obligation.cause.span, ) { - Ok(Some(valtree)) => Ok(ty::Const::new_value(selcx.tcx(),valtree, self.tcx.type_of(unevaluated.def).instantiate(self.tcx, unevaluated.args))), - Ok(None) => { + Ok(Ok(valtree)) => Ok(ty::Const::new_value(selcx.tcx(),valtree, self.tcx.type_of(unevaluated.def).instantiate(self.tcx, unevaluated.args))), + Ok(Err(_)) => { let tcx = self.tcx; let reported = tcx.dcx().emit_err(UnableToConstructConstantValue { diff --git a/src/tools/clippy/clippy_lints/src/non_copy_const.rs b/src/tools/clippy/clippy_lints/src/non_copy_const.rs index 09225ac3246cb..6f5505e8a6387 100644 --- a/src/tools/clippy/clippy_lints/src/non_copy_const.rs +++ b/src/tools/clippy/clippy_lints/src/non_copy_const.rs @@ -235,7 +235,7 @@ impl<'tcx> NonCopyConst<'tcx> { fn is_value_unfrozen_raw( cx: &LateContext<'tcx>, - result: Result>, ErrorHandled>, + result: Result, Ty<'tcx>>, ErrorHandled>, ty: Ty<'tcx>, ) -> bool { result.map_or_else( From e613bc92a12c176d0e206b2f83b1bae8011e90c1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 13 Jul 2024 17:24:50 +0200 Subject: [PATCH 17/20] const_to_pat: cleanup leftovers from when we had to deal with non-structural constants --- Cargo.lock | 1 + compiler/rustc_middle/src/ty/consts.rs | 52 +++-- compiler/rustc_mir_build/Cargo.toml | 1 + .../src/thir/pattern/const_to_pat.rs | 211 ++++++------------ .../rustc_mir_build/src/thir/pattern/mod.rs | 122 +++------- .../rustc_trait_selection/src/traits/mod.rs | 2 - .../src/traits/structural_match.rs | 173 -------------- .../const_in_pattern/custom-eq-branch-pass.rs | 11 + .../const_in_pattern/reject_non_partial_eq.rs | 2 +- .../reject_non_partial_eq.stderr | 5 +- .../invalid-inline-const-in-match-arm.rs | 1 + .../invalid-inline-const-in-match-arm.stderr | 8 +- ...-61188-match-slice-forbidden-without-eq.rs | 2 +- ...88-match-slice-forbidden-without-eq.stderr | 5 +- .../issue-6804-nan-match.rs | 4 - .../issue-6804-nan-match.stderr | 34 +-- 16 files changed, 174 insertions(+), 460 deletions(-) delete mode 100644 compiler/rustc_trait_selection/src/traits/structural_match.rs diff --git a/Cargo.lock b/Cargo.lock index 2d3bc59dddb21..1c1607e4c1ac4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4411,6 +4411,7 @@ dependencies = [ name = "rustc_mir_build" version = "0.0.0" dependencies = [ + "either", "itertools", "rustc_apfloat", "rustc_arena", diff --git a/compiler/rustc_middle/src/ty/consts.rs b/compiler/rustc_middle/src/ty/consts.rs index d4c39a25a1f28..4d213d14af126 100644 --- a/compiler/rustc_middle/src/ty/consts.rs +++ b/compiler/rustc_middle/src/ty/consts.rs @@ -1,6 +1,7 @@ use crate::middle::resolve_bound_vars as rbv; use crate::mir::interpret::{ErrorHandled, LitToConstInput, Scalar}; use crate::ty::{self, GenericArgs, ParamEnv, ParamEnvAnd, Ty, TyCtxt, TypeVisitableExt}; +use either::Either; use rustc_data_structures::intern::Interned; use rustc_error_messages::MultiSpan; use rustc_hir as hir; @@ -312,14 +313,16 @@ impl<'tcx> Const<'tcx> { Self::from_bits(tcx, n as u128, ParamEnv::empty().and(tcx.types.usize)) } - /// Returns the evaluated constant + /// Returns the evaluated constant as a valtree; + /// if that fails due to a valtree-incompatible type, indicate which type that is + /// by returning `Err(Left(bad_type))`. #[inline] - pub fn eval( + pub fn eval_valtree( self, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, span: Span, - ) -> Result<(Ty<'tcx>, ValTree<'tcx>), ErrorHandled> { + ) -> Result<(Ty<'tcx>, ValTree<'tcx>), Either, ErrorHandled>> { assert!(!self.has_escaping_bound_vars(), "escaping vars in {self:?}"); match self.kind() { ConstKind::Unevaluated(unevaluated) => { @@ -328,26 +331,47 @@ impl<'tcx> Const<'tcx> { let (param_env, unevaluated) = unevaluated.prepare_for_eval(tcx, param_env); // try to resolve e.g. associated constants to their definition on an impl, and then // evaluate the const. - let Ok(c) = tcx.const_eval_resolve_for_typeck(param_env, unevaluated, span)? else { - // This can happen when we run on ill-typed code. - let e = tcx.dcx().span_delayed_bug( - span, - "`ty::Const::eval` called on a non-valtree-compatible type", - ); - return Err(e.into()); - }; - Ok((tcx.type_of(unevaluated.def).instantiate(tcx, unevaluated.args), c)) + match tcx.const_eval_resolve_for_typeck(param_env, unevaluated, span) { + Ok(Ok(c)) => { + Ok((tcx.type_of(unevaluated.def).instantiate(tcx, unevaluated.args), c)) + } + Ok(Err(bad_ty)) => Err(Either::Left(bad_ty)), + Err(err) => Err(Either::Right(err.into())), + } } ConstKind::Value(ty, val) => Ok((ty, val)), - ConstKind::Error(g) => Err(g.into()), + ConstKind::Error(g) => Err(Either::Right(g.into())), ConstKind::Param(_) | ConstKind::Infer(_) | ConstKind::Bound(_, _) | ConstKind::Placeholder(_) - | ConstKind::Expr(_) => Err(ErrorHandled::TooGeneric(span)), + | ConstKind::Expr(_) => Err(Either::Right(ErrorHandled::TooGeneric(span))), } } + /// Returns the evaluated constant + #[inline] + pub fn eval( + self, + tcx: TyCtxt<'tcx>, + param_env: ParamEnv<'tcx>, + span: Span, + ) -> Result<(Ty<'tcx>, ValTree<'tcx>), ErrorHandled> { + self.eval_valtree(tcx, param_env, span).map_err(|err| { + match err { + Either::Right(err) => err, + Either::Left(_bad_ty) => { + // This can happen when we run on ill-typed code. + let e = tcx.dcx().span_delayed_bug( + span, + "`ty::Const::eval` called on a non-valtree-compatible type", + ); + e.into() + } + } + }) + } + /// Normalizes the constant to a value or an error if possible. #[inline] pub fn normalize(self, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>) -> Self { diff --git a/compiler/rustc_mir_build/Cargo.toml b/compiler/rustc_mir_build/Cargo.toml index 5d828d0093f3c..529e9cc271136 100644 --- a/compiler/rustc_mir_build/Cargo.toml +++ b/compiler/rustc_mir_build/Cargo.toml @@ -5,6 +5,7 @@ edition = "2021" [dependencies] # tidy-alphabetical-start +either = "1.5.0" itertools = "0.12" rustc_apfloat = "0.2.0" rustc_arena = { path = "../rustc_arena" } diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 5745dc0969cce..76efb6574e17b 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -1,42 +1,46 @@ +use either::Either; use rustc_apfloat::Float; use rustc_hir as hir; use rustc_index::Idx; use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_infer::traits::Obligation; use rustc_middle::mir; -use rustc_middle::span_bug; +use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::thir::{FieldPat, Pat, PatKind}; use rustc_middle::ty::{self, Ty, TyCtxt, ValTree}; use rustc_span::{ErrorGuaranteed, Span}; use rustc_target::abi::{FieldIdx, VariantIdx}; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; -use rustc_trait_selection::traits::{self, ObligationCause}; +use rustc_trait_selection::traits::ObligationCause; use tracing::{debug, instrument, trace}; use std::cell::Cell; use super::PatCtxt; use crate::errors::{ - InvalidPattern, NaNPattern, PointerPattern, TypeNotPartialEq, TypeNotStructural, UnionPattern, - UnsizedPattern, + ConstPatternDependsOnGenericParameter, CouldNotEvalConstPattern, InvalidPattern, NaNPattern, + PointerPattern, TypeNotPartialEq, TypeNotStructural, UnionPattern, UnsizedPattern, }; impl<'a, 'tcx> PatCtxt<'a, 'tcx> { - /// Converts an evaluated constant to a pattern (if possible). + /// Converts a constant to a pattern (if possible). /// This means aggregate values (like structs and enums) are converted /// to a pattern that matches the value (as if you'd compared via structural equality). /// - /// `cv` must be a valtree or a `mir::ConstValue`. + /// Only type system constants are supported, as we are using valtrees + /// as an intermediate step. Unfortunately those don't carry a type + /// so we have to carry one ourselves. #[instrument(level = "debug", skip(self), ret)] pub(super) fn const_to_pat( &self, - cv: mir::Const<'tcx>, + c: ty::Const<'tcx>, + ty: Ty<'tcx>, id: hir::HirId, span: Span, ) -> Box> { let infcx = self.tcx.infer_ctxt().build(); let mut convert = ConstToPat::new(self, id, span, infcx); - convert.to_pat(cv) + convert.to_pat(c, ty) } } @@ -56,12 +60,6 @@ struct ConstToPat<'tcx> { treat_byte_string_as_slice: bool, } -/// This error type signals that we encountered a non-struct-eq situation. -/// We will fall back to calling `PartialEq::eq` on such patterns, -/// and exhaustiveness checking will consider them as matching nothing. -#[derive(Debug)] -struct FallbackToOpaqueConst; - impl<'tcx> ConstToPat<'tcx> { fn new( pat_ctxt: &PatCtxt<'_, 'tcx>, @@ -91,116 +89,55 @@ impl<'tcx> ConstToPat<'tcx> { ty.is_structural_eq_shallow(self.infcx.tcx) } - fn to_pat(&mut self, cv: mir::Const<'tcx>) -> Box> { + fn to_pat(&mut self, c: ty::Const<'tcx>, ty: Ty<'tcx>) -> Box> { trace!(self.treat_byte_string_as_slice); - // This method is just a wrapper handling a validity check; the heavy lifting is - // performed by the recursive `recur` method, which is not meant to be - // invoked except by this method. - // - // once indirect_structural_match is a full fledged error, this - // level of indirection can be eliminated + let pat_from_kind = |kind| Box::new(Pat { span: self.span, ty, kind }); - let have_valtree = - matches!(cv, mir::Const::Ty(_, c) if matches!(c.kind(), ty::ConstKind::Value(_, _))); - let inlined_const_as_pat = match cv { - mir::Const::Ty(_, c) => match c.kind() { - ty::ConstKind::Param(_) - | ty::ConstKind::Infer(_) - | ty::ConstKind::Bound(_, _) - | ty::ConstKind::Placeholder(_) - | ty::ConstKind::Unevaluated(_) - | ty::ConstKind::Error(_) - | ty::ConstKind::Expr(_) => { - span_bug!(self.span, "unexpected const in `to_pat`: {:?}", c.kind()) - } - ty::ConstKind::Value(ty, valtree) => { - self.recur(valtree, ty).unwrap_or_else(|_: FallbackToOpaqueConst| { - Box::new(Pat { - span: self.span, - ty: cv.ty(), - kind: PatKind::Constant { value: cv }, - }) - }) - } - }, - mir::Const::Unevaluated(_, _) => { - span_bug!(self.span, "unevaluated const in `to_pat`: {cv:?}") + // Get a valtree. If that fails, this const is definitely not valid for use as a pattern. + let valtree = match c.eval_valtree(self.tcx(), self.param_env, self.span) { + Ok((_, valtree)) => valtree, + Err(Either::Right(e)) => { + let err = match e { + ErrorHandled::Reported(..) => { + // Let's tell the use where this failing const occurs. + self.tcx().dcx().emit_err(CouldNotEvalConstPattern { span: self.span }) + } + ErrorHandled::TooGeneric(_) => self + .tcx() + .dcx() + .emit_err(ConstPatternDependsOnGenericParameter { span: self.span }), + }; + return pat_from_kind(PatKind::Error(err)); + } + Err(Either::Left(bad_ty)) => { + // The pattern cannot be turned into a valtree. + let e = match bad_ty.kind() { + ty::Adt(def, ..) => { + assert!(def.is_union()); + self.tcx().dcx().emit_err(UnionPattern { span: self.span }) + } + ty::FnPtr(..) | ty::RawPtr(..) => { + self.tcx().dcx().emit_err(PointerPattern { span: self.span }) + } + _ => self + .tcx() + .dcx() + .emit_err(InvalidPattern { span: self.span, non_sm_ty: bad_ty }), + }; + return pat_from_kind(PatKind::Error(e)); } - mir::Const::Val(_, _) => Box::new(Pat { - span: self.span, - ty: cv.ty(), - kind: PatKind::Constant { value: cv }, - }), }; - if self.saw_const_match_error.get().is_none() { - // If we were able to successfully convert the const to some pat (possibly with some - // lints, but no errors), double-check that all types in the const implement - // `PartialEq`. Even if we have a valtree, we may have found something - // in there with non-structural-equality, meaning we match using `PartialEq` - // and we hence have to check if that impl exists. - // This is all messy but not worth cleaning up: at some point we'll emit - // a hard error when we don't have a valtree or when we find something in - // the valtree that is not structural; then this can all be made a lot simpler. - - let structural = traits::search_for_structural_match_violation(self.tcx(), cv.ty()); - debug!( - "search_for_structural_match_violation cv.ty: {:?} returned: {:?}", - cv.ty(), - structural - ); - - if let Some(non_sm_ty) = structural { - if !self.type_has_partial_eq_impl(cv.ty()) { - // This is reachable and important even if we have a valtree: there might be - // non-structural things in a valtree, in which case we fall back to `PartialEq` - // comparison, in which case we better make sure the trait is implemented for - // each inner type (and not just for the surrounding type). - let e = if let ty::Adt(def, ..) = non_sm_ty.kind() { - if def.is_union() { - let err = UnionPattern { span: self.span }; - self.tcx().dcx().emit_err(err) - } else { - // fatal avoids ICE from resolution of nonexistent method (rare case). - self.tcx() - .dcx() - .emit_fatal(TypeNotStructural { span: self.span, non_sm_ty }) - } - } else { - let err = InvalidPattern { span: self.span, non_sm_ty }; - self.tcx().dcx().emit_err(err) - }; - // All branches above emitted an error. Don't print any more lints. - // We errored. Signal that in the pattern, so that follow up errors can be silenced. - let kind = PatKind::Error(e); - return Box::new(Pat { span: self.span, ty: cv.ty(), kind }); - } else if !have_valtree { - // Not being structural prevented us from constructing a valtree, - // so this is definitely a case we want to reject. - let err = TypeNotStructural { span: self.span, non_sm_ty }; - let e = self.tcx().dcx().emit_err(err); - let kind = PatKind::Error(e); - return Box::new(Pat { span: self.span, ty: cv.ty(), kind }); - } else { - // This could be a violation in an inactive enum variant. - // Since we have a valtree, we trust that we have traversed the full valtree and - // complained about structural match violations there, so we don't - // have to check anything any more. - } - } else if !have_valtree { - // The only way valtree construction can fail without the structural match - // checker finding a violation is if there is a pointer somewhere. - let e = self.tcx().dcx().emit_err(PointerPattern { span: self.span }); - let kind = PatKind::Error(e); - return Box::new(Pat { span: self.span, ty: cv.ty(), kind }); - } + // Convert the valtree to a const. + let inlined_const_as_pat = self.valtree_to_pat(valtree, ty); + if self.saw_const_match_error.get().is_none() { // Always check for `PartialEq` if we had no other errors yet. - if !self.type_has_partial_eq_impl(cv.ty()) { - let err = TypeNotPartialEq { span: self.span, non_peq_ty: cv.ty() }; + if !self.type_has_partial_eq_impl(ty) { + let err = TypeNotPartialEq { span: self.span, non_peq_ty: ty }; let e = self.tcx().dcx().emit_err(err); let kind = PatKind::Error(e); - return Box::new(Pat { span: self.span, ty: cv.ty(), kind }); + return Box::new(Pat { span: self.span, ty: ty, kind }); } } @@ -243,36 +180,28 @@ impl<'tcx> ConstToPat<'tcx> { fn field_pats( &self, vals: impl Iterator, Ty<'tcx>)>, - ) -> Result>, FallbackToOpaqueConst> { + ) -> Vec> { vals.enumerate() .map(|(idx, (val, ty))| { let field = FieldIdx::new(idx); // Patterns can only use monomorphic types. let ty = self.tcx().normalize_erasing_regions(self.param_env, ty); - Ok(FieldPat { field, pattern: self.recur(val, ty)? }) + FieldPat { field, pattern: self.valtree_to_pat(val, ty) } }) .collect() } // Recursive helper for `to_pat`; invoke that (instead of calling this directly). #[instrument(skip(self), level = "debug")] - fn recur( - &self, - cv: ValTree<'tcx>, - ty: Ty<'tcx>, - ) -> Result>, FallbackToOpaqueConst> { + fn valtree_to_pat(&self, cv: ValTree<'tcx>, ty: Ty<'tcx>) -> Box> { let span = self.span; let tcx = self.tcx(); let param_env = self.param_env; let kind = match ty.kind() { - ty::FnDef(..) => { - let e = tcx.dcx().emit_err(InvalidPattern { span, non_sm_ty: ty }); - self.saw_const_match_error.set(Some(e)); - // We errored. Signal that in the pattern, so that follow up errors can be silenced. - PatKind::Error(e) - } ty::Adt(adt_def, _) if !self.type_marked_structural(ty) => { + // Extremely important check for all ADTs! Make sure they opted-in to be used in + // patterns. debug!("adt_def {:?} has !type_marked_structural for cv.ty: {:?}", adt_def, ty,); let err = TypeNotStructural { span, non_sm_ty: ty }; let e = tcx.dcx().emit_err(err); @@ -294,13 +223,9 @@ impl<'tcx> ConstToPat<'tcx> { .iter() .map(|field| field.ty(self.tcx(), args)), ), - )?, + ), } } - ty::Tuple(fields) => PatKind::Leaf { - subpatterns: self - .field_pats(cv.unwrap_branch().iter().copied().zip(fields.iter()))?, - }, ty::Adt(def, args) => { assert!(!def.is_union()); // Valtree construction would never succeed for unions. PatKind::Leaf { @@ -311,15 +236,18 @@ impl<'tcx> ConstToPat<'tcx> { .iter() .map(|field| field.ty(self.tcx(), args)), ), - )?, + ), } } + ty::Tuple(fields) => PatKind::Leaf { + subpatterns: self.field_pats(cv.unwrap_branch().iter().copied().zip(fields.iter())), + }, ty::Slice(elem_ty) => PatKind::Slice { prefix: cv .unwrap_branch() .iter() - .map(|val| self.recur(*val, *elem_ty)) - .collect::>()?, + .map(|val| self.valtree_to_pat(*val, *elem_ty)) + .collect(), slice: None, suffix: Box::new([]), }, @@ -327,8 +255,8 @@ impl<'tcx> ConstToPat<'tcx> { prefix: cv .unwrap_branch() .iter() - .map(|val| self.recur(*val, *elem_ty)) - .collect::>()?, + .map(|val| self.valtree_to_pat(*val, *elem_ty)) + .collect(), slice: None, suffix: Box::new([]), }, @@ -345,6 +273,7 @@ impl<'tcx> ConstToPat<'tcx> { if !pointee_ty.is_sized(tcx, param_env) && !pointee_ty.is_slice() { let err = UnsizedPattern { span, non_sm_ty: *pointee_ty }; let e = tcx.dcx().emit_err(err); + self.saw_const_match_error.set(Some(e)); // We errored. Signal that in the pattern, so that follow up errors can be silenced. PatKind::Error(e) } else { @@ -361,7 +290,7 @@ impl<'tcx> ConstToPat<'tcx> { _ => *pointee_ty, }; // References have the same valtree representation as their pointee. - let subpattern = self.recur(cv, pointee_ty)?; + let subpattern = self.valtree_to_pat(cv, pointee_ty); PatKind::Deref { subpattern } } } @@ -379,7 +308,7 @@ impl<'tcx> ConstToPat<'tcx> { // Also see . let e = tcx.dcx().emit_err(NaNPattern { span }); self.saw_const_match_error.set(Some(e)); - return Err(FallbackToOpaqueConst); + PatKind::Error(e) } else { PatKind::Constant { value: mir::Const::Ty(ty, ty::Const::new_value(tcx, cv, ty)), @@ -405,6 +334,6 @@ impl<'tcx> ConstToPat<'tcx> { } }; - Ok(Box::new(Pat { span, ty, kind })) + Box::new(Pat { span, ty, kind }) } } diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index 25b7d24de3979..553c5e175120f 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -14,8 +14,7 @@ use rustc_hir::pat_util::EnumerateAndAdjustIterator; use rustc_hir::{self as hir, ByRef, Mutability, RangeEnd}; use rustc_index::Idx; use rustc_lint as lint; -use rustc_middle::mir::interpret::{ErrorHandled, GlobalId, LitToConstError, LitToConstInput}; -use rustc_middle::mir::{self, Const}; +use rustc_middle::mir::interpret::{LitToConstError, LitToConstInput}; use rustc_middle::thir::{ Ascription, FieldPat, LocalVarId, Pat, PatKind, PatRange, PatRangeBoundary, }; @@ -575,63 +574,39 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { } }; - let cid = GlobalId { instance, promoted: None }; - // Prefer valtrees over opaque constants. - let const_value = self - .tcx - .const_eval_global_id_for_typeck(param_env_reveal_all, cid, span) - .map(|val| match val { - Ok(valtree) => mir::Const::Ty(ty, ty::Const::new_value(self.tcx, valtree, ty)), - Err(_) => mir::Const::Val( - self.tcx - .const_eval_global_id(param_env_reveal_all, cid, span) - .expect("const_eval_global_id_for_typeck should have already failed"), - ty, - ), - }); - - match const_value { - Ok(const_) => { - let pattern = self.const_to_pat(const_, id, span); - - if !is_associated_const { - return pattern; - } + let c = ty::Const::new_unevaluated( + self.tcx, + ty::UnevaluatedConst { def: instance.def_id(), args: instance.args }, + ); - let user_provided_types = self.typeck_results().user_provided_types(); - if let Some(&user_ty) = user_provided_types.get(id) { - let annotation = CanonicalUserTypeAnnotation { - user_ty: Box::new(user_ty), - span, - inferred_ty: self.typeck_results().node_type(id), - }; - Box::new(Pat { - span, - kind: PatKind::AscribeUserType { - subpattern: pattern, - ascription: Ascription { - annotation, - // Note that use `Contravariant` here. See the - // `variance` field documentation for details. - variance: ty::Contravariant, - }, - }, - ty: const_.ty(), - }) - } else { - pattern - } - } - Err(ErrorHandled::TooGeneric(_)) => { - // While `Reported | Linted` cases will have diagnostics emitted already - // it is not true for TooGeneric case, so we need to give user more information. - let e = self.tcx.dcx().emit_err(ConstPatternDependsOnGenericParameter { span }); - pat_from_kind(PatKind::Error(e)) - } - Err(_) => { - let e = self.tcx.dcx().emit_err(CouldNotEvalConstPattern { span }); - pat_from_kind(PatKind::Error(e)) - } + let pattern = self.const_to_pat(c, ty, id, span); + + if !is_associated_const { + return pattern; + } + + let user_provided_types = self.typeck_results().user_provided_types(); + if let Some(&user_ty) = user_provided_types.get(id) { + let annotation = CanonicalUserTypeAnnotation { + user_ty: Box::new(user_ty), + span, + inferred_ty: self.typeck_results().node_type(id), + }; + Box::new(Pat { + span, + kind: PatKind::AscribeUserType { + subpattern: pattern, + ascription: Ascription { + annotation, + // Note that use `Contravariant` here. See the + // `variance` field documentation for details. + variance: ty::Contravariant, + }, + }, + ty, + }) + } else { + pattern } } @@ -662,7 +637,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { }; if let Some(lit_input) = lit_input { match tcx.at(expr.span).lit_to_const(lit_input) { - Ok(c) => return self.const_to_pat(Const::Ty(ty, c), id, span).kind, + Ok(c) => return self.const_to_pat(c, ty, id, span).kind, // If an error occurred, ignore that it's a literal // and leave reporting the error up to const eval of // the unevaluated constant below. @@ -675,32 +650,11 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { tcx.erase_regions(ty::GenericArgs::identity_for_item(tcx, typeck_root_def_id)); let args = ty::InlineConstArgs::new(tcx, ty::InlineConstArgsParts { parent_args, ty }).args; - let uneval = mir::UnevaluatedConst { def: def_id.to_def_id(), args, promoted: None }; debug_assert!(!args.has_free_regions()); let ct = ty::UnevaluatedConst { def: def_id.to_def_id(), args }; - // First try using a valtree in order to destructure the constant into a pattern. - // FIXME: replace "try to do a thing, then fall back to another thing" - // but something more principled, like a trait query checking whether this can be turned into a valtree. - if let Ok(Ok(valtree)) = self.tcx.const_eval_resolve_for_typeck(self.param_env, ct, span) { - let subpattern = self.const_to_pat( - Const::Ty(ty, ty::Const::new_value(self.tcx, valtree, ty)), - id, - span, - ); - PatKind::InlineConstant { subpattern, def: def_id } - } else { - // If that fails, convert it to an opaque constant pattern. - match tcx.const_eval_resolve(self.param_env, uneval, span) { - Ok(val) => self.const_to_pat(mir::Const::Val(val, ty), id, span).kind, - Err(ErrorHandled::TooGeneric(_)) => { - // If we land here it means the const can't be evaluated because it's `TooGeneric`. - let e = self.tcx.dcx().emit_err(ConstPatternDependsOnGenericParameter { span }); - PatKind::Error(e) - } - Err(ErrorHandled::Reported(err, ..)) => PatKind::Error(err.into()), - } - } + let subpattern = self.const_to_pat(ty::Const::new_unevaluated(self.tcx, ct), ty, id, span); + PatKind::InlineConstant { subpattern, def: def_id } } /// Converts literals, paths and negation of literals to patterns. @@ -728,9 +682,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { let ct_ty = self.typeck_results.expr_ty(expr); let lit_input = LitToConstInput { lit: &lit.node, ty: ct_ty, neg }; match self.tcx.at(expr.span).lit_to_const(lit_input) { - Ok(constant) => { - self.const_to_pat(Const::Ty(ct_ty, constant), expr.hir_id, lit.span).kind - } + Ok(constant) => self.const_to_pat(constant, ct_ty, expr.hir_id, lit.span).kind, Err(LitToConstError::Reported(e)) => PatKind::Error(e), Err(LitToConstError::TypeError) => bug!("lower_lit: had type error"), } diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index d28982ed84925..f7eb173058299 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -16,7 +16,6 @@ pub mod query; #[allow(hidden_glob_reexports)] mod select; mod specialize; -mod structural_match; mod structural_normalize; #[allow(hidden_glob_reexports)] mod util; @@ -60,7 +59,6 @@ pub use self::specialize::specialization_graph::FutureCompatOverlapErrorKind; pub use self::specialize::{ specialization_graph, translate_args, translate_args_with_cause, OverlapError, }; -pub use self::structural_match::search_for_structural_match_violation; pub use self::structural_normalize::StructurallyNormalizeExt; pub use self::util::elaborate; pub use self::util::{expand_trait_aliases, TraitAliasExpander, TraitAliasExpansionInfo}; diff --git a/compiler/rustc_trait_selection/src/traits/structural_match.rs b/compiler/rustc_trait_selection/src/traits/structural_match.rs deleted file mode 100644 index d4535db951e25..0000000000000 --- a/compiler/rustc_trait_selection/src/traits/structural_match.rs +++ /dev/null @@ -1,173 +0,0 @@ -use rustc_data_structures::fx::FxHashSet; -use rustc_hir as hir; -use rustc_middle::bug; -use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor}; -use std::ops::ControlFlow; - -/// This method traverses the structure of `ty`, trying to find an -/// instance of an ADT (i.e. struct or enum) that doesn't implement -/// the structural-match traits, or a generic type parameter -/// (which cannot be determined to be structural-match). -/// -/// The "structure of a type" includes all components that would be -/// considered when doing a pattern match on a constant of that -/// type. -/// -/// * This means this method descends into fields of structs/enums, -/// and also descends into the inner type `T` of `&T` and `&mut T` -/// -/// * The traversal doesn't dereference unsafe pointers (`*const T`, -/// `*mut T`), and it does not visit the type arguments of an -/// instantiated generic like `PhantomData`. -/// -/// The reason we do this search is Rust currently require all ADTs -/// reachable from a constant's type to implement the -/// structural-match traits, which essentially say that -/// the implementation of `PartialEq::eq` behaves *equivalently* to a -/// comparison against the unfolded structure. -/// -/// For more background on why Rust has this requirement, and issues -/// that arose when the requirement was not enforced completely, see -/// Rust RFC 1445, rust-lang/rust#61188, and rust-lang/rust#62307. -pub fn search_for_structural_match_violation<'tcx>( - tcx: TyCtxt<'tcx>, - ty: Ty<'tcx>, -) -> Option> { - ty.visit_with(&mut Search { tcx, seen: FxHashSet::default() }).break_value() -} - -/// This implements the traversal over the structure of a given type to try to -/// find instances of ADTs (specifically structs or enums) that do not implement -/// `StructuralPartialEq`. -struct Search<'tcx> { - tcx: TyCtxt<'tcx>, - - /// Tracks ADTs previously encountered during search, so that - /// we will not recur on them again. - seen: FxHashSet, -} - -impl<'tcx> Search<'tcx> { - fn type_marked_structural(&self, adt_ty: Ty<'tcx>) -> bool { - adt_ty.is_structural_eq_shallow(self.tcx) - } -} - -impl<'tcx> TypeVisitor> for Search<'tcx> { - type Result = ControlFlow>; - - fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result { - debug!("Search visiting ty: {:?}", ty); - - let (adt_def, args) = match *ty.kind() { - ty::Adt(adt_def, args) => (adt_def, args), - ty::Param(_) => { - return ControlFlow::Break(ty); - } - ty::Dynamic(..) => { - return ControlFlow::Break(ty); - } - ty::Foreign(_) => { - return ControlFlow::Break(ty); - } - ty::Alias(..) => { - return ControlFlow::Break(ty); - } - ty::Closure(..) => { - return ControlFlow::Break(ty); - } - ty::CoroutineClosure(..) => { - return ControlFlow::Break(ty); - } - ty::Coroutine(..) | ty::CoroutineWitness(..) => { - return ControlFlow::Break(ty); - } - ty::FnDef(..) => { - // Types of formals and return in `fn(_) -> _` are also irrelevant; - // so we do not recur into them via `super_visit_with` - return ControlFlow::Continue(()); - } - ty::Array(_, n) - if { n.try_eval_target_usize(self.tcx, ty::ParamEnv::reveal_all()) == Some(0) } => - { - // rust-lang/rust#62336: ignore type of contents - // for empty array. - return ControlFlow::Continue(()); - } - ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Str | ty::Never => { - // These primitive types are always structural match. - // - // `Never` is kind of special here, but as it is not inhabitable, this should be fine. - return ControlFlow::Continue(()); - } - - ty::FnPtr(..) => { - return ControlFlow::Continue(()); - } - - ty::RawPtr(..) => { - // structural-match ignores substructure of - // `*const _`/`*mut _`, so skip `super_visit_with`. - // - // For example, if you have: - // ``` - // struct NonStructural; - // #[derive(PartialEq, Eq)] - // struct T(*const NonStructural); - // const C: T = T(std::ptr::null()); - // ``` - // - // Even though `NonStructural` does not implement `PartialEq`, - // structural equality on `T` does not recur into the raw - // pointer. Therefore, one can still use `C` in a pattern. - return ControlFlow::Continue(()); - } - - ty::Float(_) => { - return ControlFlow::Continue(()); - } - - ty::Pat(..) | ty::Array(..) | ty::Slice(_) | ty::Ref(..) | ty::Tuple(..) => { - // First check all contained types and then tell the caller to continue searching. - return ty.super_visit_with(self); - } - ty::Infer(_) | ty::Placeholder(_) | ty::Bound(..) => { - bug!("unexpected type during structural-match checking: {:?}", ty); - } - ty::Error(_) => { - // We still want to check other types after encountering an error, - // as this may still emit relevant errors. - return ControlFlow::Continue(()); - } - }; - - if !self.seen.insert(adt_def.did()) { - debug!("Search already seen adt_def: {:?}", adt_def); - return ControlFlow::Continue(()); - } - - if !self.type_marked_structural(ty) { - debug!("Search found ty: {:?}", ty); - return ControlFlow::Break(ty); - } - - // structural-match does not care about the - // instantiation of the generics in an ADT (it - // instead looks directly at its fields outside - // this match), so we skip super_visit_with. - // - // (Must not recur on args for `PhantomData` cf - // rust-lang/rust#55028 and rust-lang/rust#55837; but also - // want to skip args when only uses of generic are - // behind unsafe pointers `*const T`/`*mut T`.) - - // even though we skip super_visit_with, we must recur on - // fields of ADT. - let tcx = self.tcx; - adt_def.all_fields().map(|field| field.ty(tcx, args)).try_for_each(|field_ty| { - let ty = self.tcx.normalize_erasing_regions(ty::ParamEnv::empty(), field_ty); - debug!("structural-match ADT: field_ty={:?}, ty={:?}", field_ty, ty); - ty.visit_with(self) - }) - } -} diff --git a/tests/ui/consts/const_in_pattern/custom-eq-branch-pass.rs b/tests/ui/consts/const_in_pattern/custom-eq-branch-pass.rs index 2e7061e7c4b49..ab8eec876bcbb 100644 --- a/tests/ui/consts/const_in_pattern/custom-eq-branch-pass.rs +++ b/tests/ui/consts/const_in_pattern/custom-eq-branch-pass.rs @@ -23,9 +23,20 @@ const BAR_BAZ: Foo = if 42 == 42 { Foo::Qux(CustomEq) // dead arm }; +const EMPTY: &[CustomEq] = &[]; + fn main() { + // BAR_BAZ itself is fine but the enum has other variants + // that are non-structural. Still, this should be accepted. match Foo::Qux(CustomEq) { BAR_BAZ => panic!(), _ => {} } + + // Similarly, an empty slice of a type that is non-structural + // is accepted. + match &[CustomEq] as &[CustomEq] { + EMPTY => panic!(), + _ => {}, + } } diff --git a/tests/ui/consts/const_in_pattern/reject_non_partial_eq.rs b/tests/ui/consts/const_in_pattern/reject_non_partial_eq.rs index 86d971044fe1c..645e141891243 100644 --- a/tests/ui/consts/const_in_pattern/reject_non_partial_eq.rs +++ b/tests/ui/consts/const_in_pattern/reject_non_partial_eq.rs @@ -26,7 +26,7 @@ fn main() { match None { NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"), - //~^ ERROR must be annotated with `#[derive(PartialEq)]` + //~^ ERROR must implement `PartialEq` _ => panic!("whoops"), } } diff --git a/tests/ui/consts/const_in_pattern/reject_non_partial_eq.stderr b/tests/ui/consts/const_in_pattern/reject_non_partial_eq.stderr index 88b82d5004b5c..ed531a1fead8d 100644 --- a/tests/ui/consts/const_in_pattern/reject_non_partial_eq.stderr +++ b/tests/ui/consts/const_in_pattern/reject_non_partial_eq.stderr @@ -1,11 +1,8 @@ -error: to use a constant of type `NoPartialEq` in a pattern, `NoPartialEq` must be annotated with `#[derive(PartialEq)]` +error: to use a constant of type `Option` in a pattern, the type must implement `PartialEq` --> $DIR/reject_non_partial_eq.rs:28:9 | LL | NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"), | ^^^^^^^^^^^^^^^^^^ - | - = note: the traits must be derived, manual `impl`s are not sufficient - = note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details error: aborting due to 1 previous error diff --git a/tests/ui/consts/invalid-inline-const-in-match-arm.rs b/tests/ui/consts/invalid-inline-const-in-match-arm.rs index 0654fd82fbc59..4fe4b0d33c886 100644 --- a/tests/ui/consts/invalid-inline-const-in-match-arm.rs +++ b/tests/ui/consts/invalid-inline-const-in-match-arm.rs @@ -4,5 +4,6 @@ fn main() { match () { const { (|| {})() } => {} //~^ ERROR cannot call non-const closure in constants + //~| ERROR could not evaluate constant pattern } } diff --git a/tests/ui/consts/invalid-inline-const-in-match-arm.stderr b/tests/ui/consts/invalid-inline-const-in-match-arm.stderr index 7579f7f969245..0e41053a29df0 100644 --- a/tests/ui/consts/invalid-inline-const-in-match-arm.stderr +++ b/tests/ui/consts/invalid-inline-const-in-match-arm.stderr @@ -11,6 +11,12 @@ help: add `#![feature(const_trait_impl)]` to the crate attributes to enable LL + #![feature(const_trait_impl)] | -error: aborting due to 1 previous error +error: could not evaluate constant pattern + --> $DIR/invalid-inline-const-in-match-arm.rs:5:9 + | +LL | const { (|| {})() } => {} + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0015`. diff --git a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-61188-match-slice-forbidden-without-eq.rs b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-61188-match-slice-forbidden-without-eq.rs index c69fe145f77ba..c01f8934c750e 100644 --- a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-61188-match-slice-forbidden-without-eq.rs +++ b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-61188-match-slice-forbidden-without-eq.rs @@ -13,7 +13,7 @@ const A: &[B] = &[]; pub fn main() { match &[][..] { A => (), - //~^ ERROR must be annotated with `#[derive(PartialEq)]` + //~^ ERROR must implement `PartialEq` _ => (), } } diff --git a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-61188-match-slice-forbidden-without-eq.stderr b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-61188-match-slice-forbidden-without-eq.stderr index 02e2bab4d0a29..736e4c30c8ae3 100644 --- a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-61188-match-slice-forbidden-without-eq.stderr +++ b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-61188-match-slice-forbidden-without-eq.stderr @@ -1,11 +1,8 @@ -error: to use a constant of type `B` in a pattern, `B` must be annotated with `#[derive(PartialEq)]` +error: to use a constant of type `&[B]` in a pattern, the type must implement `PartialEq` --> $DIR/issue-61188-match-slice-forbidden-without-eq.rs:15:9 | LL | A => (), | ^ - | - = note: the traits must be derived, manual `impl`s are not sufficient - = note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details error: aborting due to 1 previous error diff --git a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-6804-nan-match.rs b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-6804-nan-match.rs index 6d6a336e6885b..f343013f2b084 100644 --- a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-6804-nan-match.rs +++ b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-6804-nan-match.rs @@ -28,13 +28,9 @@ fn main() { // Also cover range patterns match x { NAN..=1.0 => {}, //~ ERROR cannot use NaN in patterns - //~^ ERROR lower range bound must be less than or equal to upper -1.0..=NAN => {}, //~ ERROR cannot use NaN in patterns - //~^ ERROR lower range bound must be less than or equal to upper NAN.. => {}, //~ ERROR cannot use NaN in patterns - //~^ ERROR lower range bound must be less than or equal to upper ..NAN => {}, //~ ERROR cannot use NaN in patterns - //~^ ERROR lower range bound must be less than upper _ => {}, }; } diff --git a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-6804-nan-match.stderr b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-6804-nan-match.stderr index baca1d75048ee..44b05ea31e962 100644 --- a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-6804-nan-match.stderr +++ b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-6804-nan-match.stderr @@ -34,14 +34,8 @@ LL | NAN..=1.0 => {}, = note: NaNs compare inequal to everything, even themselves, so this pattern would never match = help: try using the `is_nan` method instead -error[E0030]: lower range bound must be less than or equal to upper - --> $DIR/issue-6804-nan-match.rs:30:9 - | -LL | NAN..=1.0 => {}, - | ^^^^^^^^^ lower bound larger than upper bound - error: cannot use NaN in patterns - --> $DIR/issue-6804-nan-match.rs:32:16 + --> $DIR/issue-6804-nan-match.rs:31:16 | LL | -1.0..=NAN => {}, | ^^^ @@ -49,14 +43,8 @@ LL | -1.0..=NAN => {}, = note: NaNs compare inequal to everything, even themselves, so this pattern would never match = help: try using the `is_nan` method instead -error[E0030]: lower range bound must be less than or equal to upper - --> $DIR/issue-6804-nan-match.rs:32:9 - | -LL | -1.0..=NAN => {}, - | ^^^^^^^^^^ lower bound larger than upper bound - error: cannot use NaN in patterns - --> $DIR/issue-6804-nan-match.rs:34:9 + --> $DIR/issue-6804-nan-match.rs:32:9 | LL | NAN.. => {}, | ^^^ @@ -64,14 +52,8 @@ LL | NAN.. => {}, = note: NaNs compare inequal to everything, even themselves, so this pattern would never match = help: try using the `is_nan` method instead -error[E0030]: lower range bound must be less than or equal to upper - --> $DIR/issue-6804-nan-match.rs:34:9 - | -LL | NAN.. => {}, - | ^^^^^ lower bound larger than upper bound - error: cannot use NaN in patterns - --> $DIR/issue-6804-nan-match.rs:36:11 + --> $DIR/issue-6804-nan-match.rs:33:11 | LL | ..NAN => {}, | ^^^ @@ -79,13 +61,5 @@ LL | ..NAN => {}, = note: NaNs compare inequal to everything, even themselves, so this pattern would never match = help: try using the `is_nan` method instead -error[E0579]: lower range bound must be less than upper - --> $DIR/issue-6804-nan-match.rs:36:9 - | -LL | ..NAN => {}, - | ^^^^^ - -error: aborting due to 11 previous errors +error: aborting due to 7 previous errors -Some errors have detailed explanations: E0030, E0579. -For more information about an error, try `rustc --explain E0030`. From 86ce911f90d9498b33738655c8cfed2f637c335a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 13 Jul 2024 18:03:05 +0200 Subject: [PATCH 18/20] pattern lowering: make sure we never call user-defined PartialEq instances --- compiler/rustc_middle/src/thir.rs | 11 ++--- .../rustc_mir_build/src/build/matches/test.rs | 45 +++++++------------ compiler/rustc_pattern_analysis/src/rustc.rs | 18 ++++++-- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index c97af68c29e5f..b80d00719ee5e 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -783,16 +783,13 @@ pub enum PatKind<'tcx> { }, /// One of the following: - /// * `&str` (represented as a valtree), which will be handled as a string pattern and thus - /// exhaustiveness checking will detect if you use the same string twice in different - /// patterns. + /// * `&str`/`&[u8]` (represented as a valtree), which will be handled as a string/slice pattern + /// and thus exhaustiveness checking will detect if you use the same string/slice twice in + /// different patterns. /// * integer, bool, char or float (represented as a valtree), which will be handled by /// exhaustiveness to cover exactly its own value, similar to `&str`, but these values are /// much simpler. - /// * Opaque constants (represented as `mir::ConstValue`), that must not be matched - /// structurally. So anything that does not derive `PartialEq` and `Eq`. - /// - /// These are always compared with the matched place using (the semantics of) `PartialEq`. + /// * `String`, if `string_deref_patterns` is enabled. Constant { value: mir::Const<'tcx>, }, diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index d29874a5ad4ba..5aed2537750dd 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -144,7 +144,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { && tcx.is_lang_item(def.did(), LangItem::String) { if !tcx.features().string_deref_patterns { - bug!( + span_bug!( + test.span, "matching on `String` went through without enabling string_deref_patterns" ); } @@ -432,40 +433,28 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } - match *ty.kind() { - ty::Ref(_, deref_ty, _) => ty = deref_ty, - _ => { - // non_scalar_compare called on non-reference type - let temp = self.temp(ty, source_info.span); - self.cfg.push_assign(block, source_info, temp, Rvalue::Use(expect)); - let ref_ty = Ty::new_imm_ref(self.tcx, self.tcx.lifetimes.re_erased, ty); - let ref_temp = self.temp(ref_ty, source_info.span); - - self.cfg.push_assign( - block, - source_info, - ref_temp, - Rvalue::Ref(self.tcx.lifetimes.re_erased, BorrowKind::Shared, temp), - ); - expect = Operand::Move(ref_temp); - - let ref_temp = self.temp(ref_ty, source_info.span); - self.cfg.push_assign( - block, - source_info, - ref_temp, - Rvalue::Ref(self.tcx.lifetimes.re_erased, BorrowKind::Shared, val), - ); - val = ref_temp; + // Figure out the type on which we are calling `PartialEq`. This involves an extra wrapping + // reference: we can only compare two `&T`, and then compare_ty will be `T`. + // Make sure that we do *not* call any user-defined code here. + // The only types that can end up here are string and byte literals, + // which have their comparison defined in `core`. + // (Interestingly this means that exhaustiveness analysis relies, for soundness, + // on the `PartialEq` impls for `str` and `[u8]` to b correct!) + let compare_ty = match *ty.kind() { + ty::Ref(_, deref_ty, _) + if deref_ty == self.tcx.types.str_ || deref_ty != self.tcx.types.u8 => + { + deref_ty } - } + _ => span_bug!(source_info.span, "invalid type for non-scalar compare: {}", ty), + }; let eq_def_id = self.tcx.require_lang_item(LangItem::PartialEq, Some(source_info.span)); let method = trait_method( self.tcx, eq_def_id, sym::eq, - self.tcx.with_opt_host_effect_param(self.def_id, eq_def_id, [ty, ty]), + self.tcx.with_opt_host_effect_param(self.def_id, eq_def_id, [compare_ty, compare_ty]), ); let bool_ty = self.tcx.types.bool; diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index d4dd4dd858c28..d17ee8bff503e 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -462,7 +462,12 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { // This is a box pattern. ty::Adt(adt, ..) if adt.is_box() => Struct, ty::Ref(..) => Ref, - _ => bug!("pattern has unexpected type: pat: {:?}, ty: {:?}", pat, ty), + _ => span_bug!( + pat.span, + "pattern has unexpected type: pat: {:?}, ty: {:?}", + pat.kind, + ty.inner() + ), }; } PatKind::DerefPattern { .. } => { @@ -518,7 +523,12 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { .map(|ipat| self.lower_pat(&ipat.pattern).at_index(ipat.field.index())) .collect(); } - _ => bug!("pattern has unexpected type: pat: {:?}, ty: {:?}", pat, ty), + _ => span_bug!( + pat.span, + "pattern has unexpected type: pat: {:?}, ty: {}", + pat.kind, + ty.inner() + ), } } PatKind::Constant { value } => { @@ -663,7 +673,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { } } } - _ => bug!("invalid type for range pattern: {}", ty.inner()), + _ => span_bug!(pat.span, "invalid type for range pattern: {}", ty.inner()), }; fields = vec![]; arity = 0; @@ -674,7 +684,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { Some(length.eval_target_usize(cx.tcx, cx.param_env) as usize) } ty::Slice(_) => None, - _ => span_bug!(pat.span, "bad ty {:?} for slice pattern", ty), + _ => span_bug!(pat.span, "bad ty {} for slice pattern", ty.inner()), }; let kind = if slice.is_some() { SliceKind::VarLen(prefix.len(), suffix.len()) From 67c99d6338922d270108fc6390d3ad5224b3a7e1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 13 Jul 2024 18:32:10 +0200 Subject: [PATCH 19/20] avoid creating an Instance only to immediately disassemble it again --- .../src/error_codes/E0158.md | 17 +++++----- compiler/rustc_mir_build/messages.ftl | 2 -- compiler/rustc_mir_build/src/errors.rs | 9 +---- .../rustc_mir_build/src/thir/pattern/mod.rs | 33 ++----------------- src/tools/tidy/src/issues.txt | 1 - ...ssociated-const-type-parameter-arms.stderr | 15 --------- ...ssociated-const-type-parameter-pattern.rs} | 10 ++++-- ...ciated-const-type-parameter-pattern.stderr | 27 +++++++++++++++ .../ui/consts/issue-73976-polymorphic.stderr | 5 +-- tests/ui/consts/issue-79137-toogeneric.stderr | 3 +- .../const-match-pat-generic.stderr | 5 +-- .../issue-68393-let-pat-assoc-constant.rs | 26 --------------- .../issue-68393-let-pat-assoc-constant.stderr | 15 --------- 13 files changed, 54 insertions(+), 114 deletions(-) delete mode 100644 tests/ui/associated-consts/associated-const-type-parameter-arms.stderr rename tests/ui/associated-consts/{associated-const-type-parameter-arms.rs => associated-const-type-parameter-pattern.rs} (50%) create mode 100644 tests/ui/associated-consts/associated-const-type-parameter-pattern.stderr delete mode 100644 tests/ui/pattern/issue-68393-let-pat-assoc-constant.rs delete mode 100644 tests/ui/pattern/issue-68393-let-pat-assoc-constant.stderr diff --git a/compiler/rustc_error_codes/src/error_codes/E0158.md b/compiler/rustc_error_codes/src/error_codes/E0158.md index 03b93d925c19a..c31f1e13beee4 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0158.md +++ b/compiler/rustc_error_codes/src/error_codes/E0158.md @@ -1,5 +1,4 @@ -An associated `const`, `const` parameter or `static` has been referenced -in a pattern. +A generic parameter or `static` has been referenced in a pattern. Erroneous code example: @@ -15,25 +14,25 @@ trait Bar { fn test(arg: Foo) { match arg { - A::X => println!("A::X"), // error: E0158: associated consts cannot be - // referenced in patterns + A::X => println!("A::X"), // error: E0158: constant pattern depends + // on a generic parameter Foo::Two => println!("Two") } } ``` -Associated `const`s cannot be referenced in patterns because it is impossible +Generic parameters cannot be referenced in patterns because it is impossible for the compiler to prove exhaustiveness (that some pattern will always match). Take the above example, because Rust does type checking in the *generic* method, not the *monomorphized* specific instance. So because `Bar` could have -theoretically infinite implementations, there's no way to always be sure that +theoretically arbitrary implementations, there's no way to always be sure that `A::X` is `Foo::One`. So this code must be rejected. Even if code can be proven exhaustive by a programmer, the compiler cannot currently prove this. -The same holds true of `const` parameters and `static`s. +The same holds true of `static`s. -If you want to match against an associated `const`, `const` parameter or -`static` consider using a guard instead: +If you want to match against a `const` that depends on a generic parameter or a +`static`, consider using a guard instead: ``` trait Trait { diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 0c277811fdacf..281f3ef6ef35a 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -4,8 +4,6 @@ mir_build_already_borrowed = cannot borrow value as mutable because it is also b mir_build_already_mut_borrowed = cannot borrow value as immutable because it is also borrowed as mutable -mir_build_assoc_const_in_pattern = associated consts cannot be referenced in patterns - mir_build_bindings_with_variant_name = pattern binding `{$name}` is named the same as one of the variants of the type `{$ty_path}` .suggestion = to match on the variant, qualify the path diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 7c73d8a6d47d4..f6f443b64a63a 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -566,13 +566,6 @@ pub(crate) struct StaticInPattern { pub(crate) span: Span, } -#[derive(Diagnostic)] -#[diag(mir_build_assoc_const_in_pattern, code = E0158)] -pub(crate) struct AssocConstInPattern { - #[primary_span] - pub(crate) span: Span, -} - #[derive(Diagnostic)] #[diag(mir_build_const_param_in_pattern, code = E0158)] pub(crate) struct ConstParamInPattern { @@ -597,7 +590,7 @@ pub(crate) struct UnreachablePattern { } #[derive(Diagnostic)] -#[diag(mir_build_const_pattern_depends_on_generic_parameter)] +#[diag(mir_build_const_pattern_depends_on_generic_parameter, code = E0158)] pub(crate) struct ConstPatternDependsOnGenericParameter { #[primary_span] pub(crate) span: Span, diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index 553c5e175120f..622651800f44c 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -548,37 +548,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { _ => return pat_from_kind(self.lower_variant_or_leaf(res, id, span, ty, vec![])), }; - // Use `Reveal::All` here because patterns are always monomorphic even if their function - // isn't. - let param_env_reveal_all = self.param_env.with_reveal_all_normalized(self.tcx); - // N.B. There is no guarantee that args collected in typeck results are fully normalized, - // so they need to be normalized in order to pass to `Instance::resolve`, which will ICE - // if given unnormalized types. - let args = self - .tcx - .normalize_erasing_regions(param_env_reveal_all, self.typeck_results.node_args(id)); - let instance = match ty::Instance::try_resolve(self.tcx, param_env_reveal_all, def_id, args) - { - Ok(Some(i)) => i, - Ok(None) => { - // It should be assoc consts if there's no error but we cannot resolve it. - debug_assert!(is_associated_const); - - let e = self.tcx.dcx().emit_err(AssocConstInPattern { span }); - return pat_from_kind(PatKind::Error(e)); - } - - Err(_) => { - let e = self.tcx.dcx().emit_err(CouldNotEvalConstPattern { span }); - return pat_from_kind(PatKind::Error(e)); - } - }; - - let c = ty::Const::new_unevaluated( - self.tcx, - ty::UnevaluatedConst { def: instance.def_id(), args: instance.args }, - ); - + let args = self.typeck_results.node_args(id); + let c = ty::Const::new_unevaluated(self.tcx, ty::UnevaluatedConst { def: def_id, args }); let pattern = self.const_to_pat(c, ty, id, span); if !is_associated_const { diff --git a/src/tools/tidy/src/issues.txt b/src/tools/tidy/src/issues.txt index 3c7284ce6db60..5731097770481 100644 --- a/src/tools/tidy/src/issues.txt +++ b/src/tools/tidy/src/issues.txt @@ -3449,7 +3449,6 @@ ui/pattern/issue-6449.rs ui/pattern/issue-66270-pat-struct-parser-recovery.rs ui/pattern/issue-67037-pat-tup-scrut-ty-diff-less-fields.rs ui/pattern/issue-67776-match-same-name-enum-variant-refs.rs -ui/pattern/issue-68393-let-pat-assoc-constant.rs ui/pattern/issue-72565.rs ui/pattern/issue-72574-1.rs ui/pattern/issue-72574-2.rs diff --git a/tests/ui/associated-consts/associated-const-type-parameter-arms.stderr b/tests/ui/associated-consts/associated-const-type-parameter-arms.stderr deleted file mode 100644 index 1ccf9febd4bdc..0000000000000 --- a/tests/ui/associated-consts/associated-const-type-parameter-arms.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0158]: associated consts cannot be referenced in patterns - --> $DIR/associated-const-type-parameter-arms.rs:20:9 - | -LL | A::X => println!("A::X"), - | ^^^^ - -error[E0158]: associated consts cannot be referenced in patterns - --> $DIR/associated-const-type-parameter-arms.rs:22:9 - | -LL | B::X => println!("B::X"), - | ^^^^ - -error: aborting due to 2 previous errors - -For more information about this error, try `rustc --explain E0158`. diff --git a/tests/ui/associated-consts/associated-const-type-parameter-arms.rs b/tests/ui/associated-consts/associated-const-type-parameter-pattern.rs similarity index 50% rename from tests/ui/associated-consts/associated-const-type-parameter-arms.rs rename to tests/ui/associated-consts/associated-const-type-parameter-pattern.rs index 3f260d84e4c0a..b5798adc71c86 100644 --- a/tests/ui/associated-consts/associated-const-type-parameter-arms.rs +++ b/tests/ui/associated-consts/associated-const-type-parameter-pattern.rs @@ -18,12 +18,18 @@ impl Foo for Def { pub fn test(arg: EFoo) { match arg { A::X => println!("A::X"), - //~^ error: associated consts cannot be referenced in patterns [E0158] + //~^ error: constant pattern depends on a generic parameter B::X => println!("B::X"), - //~^ error: associated consts cannot be referenced in patterns [E0158] + //~^ error: constant pattern depends on a generic parameter _ => (), } } +pub fn test_let_pat(arg: EFoo, A::X: EFoo) { + //~^ ERROR constant pattern depends on a generic parameter + let A::X = arg; + //~^ ERROR constant pattern depends on a generic parameter +} + fn main() { } diff --git a/tests/ui/associated-consts/associated-const-type-parameter-pattern.stderr b/tests/ui/associated-consts/associated-const-type-parameter-pattern.stderr new file mode 100644 index 0000000000000..adc8f3992072d --- /dev/null +++ b/tests/ui/associated-consts/associated-const-type-parameter-pattern.stderr @@ -0,0 +1,27 @@ +error[E0158]: constant pattern depends on a generic parameter + --> $DIR/associated-const-type-parameter-pattern.rs:20:9 + | +LL | A::X => println!("A::X"), + | ^^^^ + +error[E0158]: constant pattern depends on a generic parameter + --> $DIR/associated-const-type-parameter-pattern.rs:22:9 + | +LL | B::X => println!("B::X"), + | ^^^^ + +error[E0158]: constant pattern depends on a generic parameter + --> $DIR/associated-const-type-parameter-pattern.rs:30:9 + | +LL | let A::X = arg; + | ^^^^ + +error[E0158]: constant pattern depends on a generic parameter + --> $DIR/associated-const-type-parameter-pattern.rs:28:48 + | +LL | pub fn test_let_pat(arg: EFoo, A::X: EFoo) { + | ^^^^ + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0158`. diff --git a/tests/ui/consts/issue-73976-polymorphic.stderr b/tests/ui/consts/issue-73976-polymorphic.stderr index 97a5fbc5747a3..8a44eb9854fef 100644 --- a/tests/ui/consts/issue-73976-polymorphic.stderr +++ b/tests/ui/consts/issue-73976-polymorphic.stderr @@ -1,10 +1,10 @@ -error: constant pattern depends on a generic parameter +error[E0158]: constant pattern depends on a generic parameter --> $DIR/issue-73976-polymorphic.rs:20:37 | LL | matches!(GetTypeId::::VALUE, GetTypeId::::VALUE) | ^^^^^^^^^^^^^^^^^^^^^ -error: constant pattern depends on a generic parameter +error[E0158]: constant pattern depends on a generic parameter --> $DIR/issue-73976-polymorphic.rs:31:42 | LL | matches!(GetTypeNameLen::::VALUE, GetTypeNameLen::::VALUE) @@ -12,3 +12,4 @@ LL | matches!(GetTypeNameLen::::VALUE, GetTypeNameLen::::VALUE) error: aborting due to 2 previous errors +For more information about this error, try `rustc --explain E0158`. diff --git a/tests/ui/consts/issue-79137-toogeneric.stderr b/tests/ui/consts/issue-79137-toogeneric.stderr index 18bdde45e2c9d..de81512ec6d2c 100644 --- a/tests/ui/consts/issue-79137-toogeneric.stderr +++ b/tests/ui/consts/issue-79137-toogeneric.stderr @@ -1,4 +1,4 @@ -error: constant pattern depends on a generic parameter +error[E0158]: constant pattern depends on a generic parameter --> $DIR/issue-79137-toogeneric.rs:12:43 | LL | matches!(GetVariantCount::::VALUE, GetVariantCount::::VALUE) @@ -6,3 +6,4 @@ LL | matches!(GetVariantCount::::VALUE, GetVariantCount::::VALUE) error: aborting due to 1 previous error +For more information about this error, try `rustc --explain E0158`. diff --git a/tests/ui/inline-const/const-match-pat-generic.stderr b/tests/ui/inline-const/const-match-pat-generic.stderr index 15c3a876afcf7..26f72b34eca29 100644 --- a/tests/ui/inline-const/const-match-pat-generic.stderr +++ b/tests/ui/inline-const/const-match-pat-generic.stderr @@ -1,10 +1,10 @@ -error: constant pattern depends on a generic parameter +error[E0158]: constant pattern depends on a generic parameter --> $DIR/const-match-pat-generic.rs:7:9 | LL | const { V } => {}, | ^^^^^^^^^^^ -error: constant pattern depends on a generic parameter +error[E0158]: constant pattern depends on a generic parameter --> $DIR/const-match-pat-generic.rs:19:9 | LL | const { f(V) } => {}, @@ -12,3 +12,4 @@ LL | const { f(V) } => {}, error: aborting due to 2 previous errors +For more information about this error, try `rustc --explain E0158`. diff --git a/tests/ui/pattern/issue-68393-let-pat-assoc-constant.rs b/tests/ui/pattern/issue-68393-let-pat-assoc-constant.rs deleted file mode 100644 index 95ead6b5d4a61..0000000000000 --- a/tests/ui/pattern/issue-68393-let-pat-assoc-constant.rs +++ /dev/null @@ -1,26 +0,0 @@ -pub enum EFoo { - A, -} - -pub trait Foo { - const X: EFoo; -} - -struct Abc; - -impl Foo for Abc { - const X: EFoo = EFoo::A; -} - -struct Def; -impl Foo for Def { - const X: EFoo = EFoo::A; -} - -pub fn test(arg: EFoo, A::X: EFoo) { - //~^ ERROR associated consts cannot be referenced in patterns - let A::X = arg; - //~^ ERROR associated consts cannot be referenced in patterns -} - -fn main() {} diff --git a/tests/ui/pattern/issue-68393-let-pat-assoc-constant.stderr b/tests/ui/pattern/issue-68393-let-pat-assoc-constant.stderr deleted file mode 100644 index 62c90b638d7c3..0000000000000 --- a/tests/ui/pattern/issue-68393-let-pat-assoc-constant.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0158]: associated consts cannot be referenced in patterns - --> $DIR/issue-68393-let-pat-assoc-constant.rs:22:9 - | -LL | let A::X = arg; - | ^^^^ - -error[E0158]: associated consts cannot be referenced in patterns - --> $DIR/issue-68393-let-pat-assoc-constant.rs:20:40 - | -LL | pub fn test(arg: EFoo, A::X: EFoo) { - | ^^^^ - -error: aborting due to 2 previous errors - -For more information about this error, try `rustc --explain E0158`. From 303a2db2360e332a29a66de94fa65cdb52b0d51c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 15 Jul 2024 22:11:21 +0200 Subject: [PATCH 20/20] remove saw_const_match_error; check if pattern contains an Error instead --- .../src/thir/pattern/const_to_pat.rs | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 76efb6574e17b..0d54f332585aa 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -7,15 +7,14 @@ use rustc_infer::traits::Obligation; use rustc_middle::mir; use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::thir::{FieldPat, Pat, PatKind}; +use rustc_middle::ty::TypeVisitableExt; use rustc_middle::ty::{self, Ty, TyCtxt, ValTree}; -use rustc_span::{ErrorGuaranteed, Span}; +use rustc_span::Span; use rustc_target::abi::{FieldIdx, VariantIdx}; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; use rustc_trait_selection::traits::ObligationCause; use tracing::{debug, instrument, trace}; -use std::cell::Cell; - use super::PatCtxt; use crate::errors::{ ConstPatternDependsOnGenericParameter, CouldNotEvalConstPattern, InvalidPattern, NaNPattern, @@ -49,11 +48,6 @@ struct ConstToPat<'tcx> { span: Span, param_env: ty::ParamEnv<'tcx>, - // This tracks if we emitted some hard error for a given const value, so that - // we will not subsequently issue an irrelevant lint for the same const - // value. - saw_const_match_error: Cell>, - // inference context used for checking `T: Structural` bounds. infcx: InferCtxt<'tcx>, @@ -73,7 +67,6 @@ impl<'tcx> ConstToPat<'tcx> { span, infcx, param_env: pat_ctxt.param_env, - saw_const_match_error: Cell::new(None), treat_byte_string_as_slice: pat_ctxt .typeck_results .treat_byte_string_as_slice @@ -131,7 +124,7 @@ impl<'tcx> ConstToPat<'tcx> { // Convert the valtree to a const. let inlined_const_as_pat = self.valtree_to_pat(valtree, ty); - if self.saw_const_match_error.get().is_none() { + if !inlined_const_as_pat.references_error() { // Always check for `PartialEq` if we had no other errors yet. if !self.type_has_partial_eq_impl(ty) { let err = TypeNotPartialEq { span: self.span, non_peq_ty: ty }; @@ -205,7 +198,6 @@ impl<'tcx> ConstToPat<'tcx> { debug!("adt_def {:?} has !type_marked_structural for cv.ty: {:?}", adt_def, ty,); let err = TypeNotStructural { span, non_sm_ty: ty }; let e = tcx.dcx().emit_err(err); - self.saw_const_match_error.set(Some(e)); // We errored. Signal that in the pattern, so that follow up errors can be silenced. PatKind::Error(e) } @@ -273,7 +265,6 @@ impl<'tcx> ConstToPat<'tcx> { if !pointee_ty.is_sized(tcx, param_env) && !pointee_ty.is_slice() { let err = UnsizedPattern { span, non_sm_ty: *pointee_ty }; let e = tcx.dcx().emit_err(err); - self.saw_const_match_error.set(Some(e)); // We errored. Signal that in the pattern, so that follow up errors can be silenced. PatKind::Error(e) } else { @@ -307,7 +298,6 @@ impl<'tcx> ConstToPat<'tcx> { // NaNs are not ever equal to anything so they make no sense as patterns. // Also see . let e = tcx.dcx().emit_err(NaNPattern { span }); - self.saw_const_match_error.set(Some(e)); PatKind::Error(e) } else { PatKind::Constant { @@ -328,7 +318,6 @@ impl<'tcx> ConstToPat<'tcx> { _ => { let err = InvalidPattern { span, non_sm_ty: ty }; let e = tcx.dcx().emit_err(err); - self.saw_const_match_error.set(Some(e)); // We errored. Signal that in the pattern, so that follow up errors can be silenced. PatKind::Error(e) }