rust icon indicating copy to clipboard operation
rust copied to clipboard

Implement `std::marker::Tuple`, use it in `extern "rust-call"` and `Fn`-family traits

Open compiler-errors opened this issue 2 years ago • 6 comments

Implements rust-lang/compiler-team#537

I made a few opinionated decisions in this implementation, specifically:

  1. Enforcing extern "rust-call" on fn items during wfcheck,
  2. Enforcing this for all functions (not just ones that have bodies),
  3. Gating this Tuple marker trait behind its own feature, instead of grouping it into (e.g.) unboxed_closures.

Still needing to be done:

  1. Enforce that extern "rust-call" fn-ptrs are well-formed only if they have 1/2 args and the second one implements Tuple. (Doing this would fix ICE in #66696.)
  2. Deny all explicit/user impls of the Tuple trait, kinda like Sized.
  3. Fixing Tuple trait built-in impl for chalk, so that chalkification tests are un-broken.

Open questions:

  1. Does this need t-lang or t-libs signoff?

Fixes #99820

compiler-errors avatar Jul 30 '22 10:07 compiler-errors

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

rustbot avatar Jul 30 '22 10:07 rustbot

r? @thomcc

(rust-highfive has picked a reviewer for you, use r? to override)

rust-highfive avatar Jul 30 '22 10:07 rust-highfive

r? compiler

compiler-errors avatar Jul 30 '22 10:07 compiler-errors

The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
---- [run-pass-valgrind] src/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call.rs stdout ----

error: compilation failed!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "-O" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call/a" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call/auxiliary"
stdout: none
--- stderr -------------------------------
warning: the feature `unsized_locals` is incomplete and may not be safe to use and/or cause compiler crashes
  |
1 | #![feature(unsized_locals)]
  |            ^^^^^^^^^^^^^^
  |
  |
  = note: `#[warn(incomplete_features)]` on by default
  = note: see issue #48055 <https://github.com/rust-lang/rust/issues/48055> for more information

error[E0277]: functions with the "rust-call" ABI must take a single non-self tuple argument
  |
  |
6 |     extern "rust-call" fn call_once(self, args: Args) -> Self::Output;
  |                           ^^^^^^^^^ `Args` is not a tuple
help: consider restricting type parameter `Args`
  |
  |
4 | pub trait FnOnce<Args: std::marker::Tuple> {

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0277`.
For more information about this error, try `rustc --explain E0277`.
------------------------------------------


---- [run-pass-valgrind] src/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call2.rs stdout ----

error: compilation failed!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call2.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "-O" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call2/a" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call2/auxiliary"
stdout: none
--- stderr -------------------------------
warning: the feature `unsized_locals` is incomplete and may not be safe to use and/or cause compiler crashes
  |
1 | #![feature(unsized_locals)]
  |            ^^^^^^^^^^^^^^
  |
  |
  = note: `#[warn(incomplete_features)]` on by default
  = note: see issue #48055 <https://github.com/rust-lang/rust/issues/48055> for more information

error[E0277]: functions with the "rust-call" ABI must take a single non-self tuple argument
  |
  |
6 |     extern "rust-call" fn call_once(self, args: Args) -> Self::Output;
  |                           ^^^^^^^^^ `Args` is not a tuple
help: consider restricting type parameter `Args`
  |
  |
4 | pub trait FnOnce<Args: std::marker::Tuple> {

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0277`.

rust-log-analyzer avatar Jul 30 '22 10:07 rust-log-analyzer

:umbrella: The latest upstream changes (presumably #99948) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Jul 30 '22 20:07 bors

This does need @rust-lang/libs-api and @rust-lang/lang review, but as long as all of it is still marked unstable, I don't think it needs an FCP.

For my part, :+1: for lang and for libs-api, for the proposed surface area. Others should chime in as well if they have thoughts or concerns, though.

joshtriplett avatar Jul 30 '22 20:07 joshtriplett

blocking on landing that other one

compiler-errors avatar Aug 10 '22 03:08 compiler-errors

Will this also fix the ICE in https://github.com/rust-lang/rust/issues/57404?

cjgillot avatar Aug 10 '22 20:08 cjgillot

Will this also fix the ICE in https://github.com/rust-lang/rust/issues/57404?

Yes, just checked.

compiler-errors avatar Aug 10 '22 21:08 compiler-errors

@cjgillot with this PR, that issue now reports:

error[E0277]: `&mut ()` is not a tuple
   --> /home/gh-compiler-errors/test.rs:6:41
    |
6   |     handlers.unwrap().as_mut().call_mut(&mut ());
    |                                -------- -^^^^^^
    |                                |        |
    |                                |        `&mut ()` is not a tuple
    |                                |        help: consider removing the leading `&`-reference
    |                                required by a bound introduced by this call
    |
    = help: the trait `Tuple` is not implemented for `&mut ()`
note: required by a bound in `call_mut`
   --> /home/gh-compiler-errors/rust/library/core/src/ops/function.rs:334:23
    |
334 | pub trait FnMut<Args: Tuple>: FnOnce<Args> {
    |                       ^^^^^ required by this bound in `call_mut`

error: aborting due to previous error

instead of ICEing.

compiler-errors avatar Aug 10 '22 21:08 compiler-errors

Now that #100251 has landed, seems this is now unblocked.

crlf0710 avatar Sep 12 '22 07:09 crlf0710

@bors try

compiler-errors avatar Oct 02 '22 07:10 compiler-errors

:hourglass: Trying commit 52655b8d45d36b35cf6b3bcc0174f7aa45406255 with merge 388b725dea7416978ab58ab1f908139d530308a7...

bors avatar Oct 02 '22 07:10 bors

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
configure: rust.debug-assertions := True
configure: rust.overflow-checks := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
Attempting with retry: make prepare
---
skip untracked path cpu-usage.csv during rustfmt invocations
skip untracked path src/doc/book/ during rustfmt invocations
skip untracked path src/doc/rust-by-example/ during rustfmt invocations
skip untracked path src/llvm-project/ during rustfmt invocations
Diff in /checkout/library/core/src/intrinsics.rs at line 2232:
     /// compile-time and at run-time. The unsafe code in crate B is fine.
     #[cfg(not(bootstrap))]
     #[rustc_const_unstable(feature = "const_eval_select", issue = "none")]
-    pub fn const_eval_select<ARG: Tuple, F, G, RET>(arg: ARG, called_in_const: F, called_at_rt: G) -> RET
+    pub fn const_eval_select<ARG: Tuple, F, G, RET>(
+        arg: ARG,
+        called_in_const: F,
+        called_at_rt: G,
     where
     where
         G: FnOnce<ARG, Output = RET>,
         F: FnOnce<ARG, Output = RET>;
Running `"/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/rustfmt" "--config-path" "/checkout" "--edition" "2021" "--unstable-features" "--skip-children" "--check" "/checkout/library/core/src/intrinsics.rs" "/checkout/library/core/src/error.rs" "/checkout/library/core/src/async_iter/mod.rs" "/checkout/library/core/src/async_iter/from_iter.rs" "/checkout/library/core/src/async_iter/async_iter.rs" "/checkout/library/core/src/const_closure.rs" "/checkout/library/core/src/slice/iter/macros.rs" "/checkout/library/core/src/panicking.rs"` failed.
If you're running `tidy`, try again with `--bless`. Or, if you just want to format code, run `./x.py fmt` instead.

rust-log-analyzer avatar Oct 02 '22 07:10 rust-log-analyzer

:sunny: Try build successful - checks-actions Build commit: 388b725dea7416978ab58ab1f908139d530308a7 (388b725dea7416978ab58ab1f908139d530308a7)

bors avatar Oct 02 '22 08:10 bors

@craterbot check

compiler-errors avatar Oct 02 '22 16:10 compiler-errors

:ok_hand: Experiment pr-99943 created and queued. :robot: Automatically detected try build 388b725dea7416978ab58ab1f908139d530308a7 :mag: You can check out the queue and this experiment's details.

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot avatar Oct 02 '22 16:10 craterbot

:construction: Experiment pr-99943 is now running

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot avatar Oct 02 '22 16:10 craterbot

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
---- [run-pass-valgrind] src/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call.rs stdout ----

error: compilation failed!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "-O" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call/a" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call/auxiliary"
stdout: none
--- stderr -------------------------------
warning: the feature `unsized_locals` is incomplete and may not be safe to use and/or cause compiler crashes
  |
1 | #![feature(unsized_locals)]
  |            ^^^^^^^^^^^^^^
  |
  |
  = note: see issue #48055 <https://github.com/rust-lang/rust/issues/48055> for more information
  = note: `#[warn(incomplete_features)]` on by default

error[E0277]: functions with the "rust-call" ABI must take a single non-self tuple argument
  |
  |
6 |     extern "rust-call" fn call_once(self, args: Args) -> Self::Output;
  |                           ^^^^^^^^^ the trait `Tuple` is not implemented for `Args`
help: consider restricting type parameter `Args`
  |
  |
4 | pub trait FnOnce<Args: std::marker::Tuple> {

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0277`.
For more information about this error, try `rustc --explain E0277`.
------------------------------------------


---- [run-pass-valgrind] src/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call2.rs stdout ----

error: compilation failed!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call2.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "-O" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call2/a" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-valgrind/unsized-locals/by-value-trait-objects-rust-call2/auxiliary"
stdout: none
--- stderr -------------------------------
warning: the feature `unsized_locals` is incomplete and may not be safe to use and/or cause compiler crashes
  |
1 | #![feature(unsized_locals)]
  |            ^^^^^^^^^^^^^^
  |
  |
  = note: see issue #48055 <https://github.com/rust-lang/rust/issues/48055> for more information
  = note: `#[warn(incomplete_features)]` on by default

error[E0277]: functions with the "rust-call" ABI must take a single non-self tuple argument
  |
  |
6 |     extern "rust-call" fn call_once(self, args: Args) -> Self::Output;
  |                           ^^^^^^^^^ the trait `Tuple` is not implemented for `Args`
help: consider restricting type parameter `Args`
  |
  |
4 | pub trait FnOnce<Args: std::marker::Tuple> {

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0277`.

rust-log-analyzer avatar Oct 02 '22 19:10 rust-log-analyzer

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
---- [codegen] src/test/codegen/avr/avr-func-addrspace.rs stdout ----

error: compilation failed!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/codegen/avr/avr-func-addrspace.rs" "-Zthreads=1" "-O" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/avr/avr-func-addrspace" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-O" "--target=avr-unknown-gnu-atmega328" "--crate-type=rlib" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/avr/avr-func-addrspace/auxiliary" "--emit=llvm-ir"
stdout: none
--- stderr -------------------------------
error: requires `tuple_trait` lang_item
   |
   |
53 |     extern "rust-call" fn call_once(self, args: A) -> R {

error: aborting due to previous error
------------------------------------------

rust-log-analyzer avatar Oct 02 '22 20:10 rust-log-analyzer

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
failures:

---- [incremental] src/test/incremental/hashes/extern_mods.rs stdout ----

error in revision `cfail2`: test compilation failed although it shouldn't!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/incremental/hashes/extern_mods.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--cfg" "cfail2" "-C" "incremental=/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental/hashes/extern_mods/extern_mods.inc" "-Z" "incremental-verify-ich" "-O" "--error-format" "json" "--json" "future-incompat" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental/hashes/extern_mods" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-Z" "query-dep-graph" "-O" "-Zincremental-ignore-spans" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental/hashes/extern_mods/auxiliary"
stdout: none
--- stderr -------------------------------
error[E0277]: functions with the "rust-call" ABI must take a single non-self tuple argument
   |
   |
LL |     pub fn change_calling_convention(c: i32);
   |            ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Tuple` is not implemented for `i32`
error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
------------------------------------------

rust-log-analyzer avatar Oct 02 '22 20:10 rust-log-analyzer

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Testing error-index stage2
doc tests for: /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md


command did not execute successfully: "/checkout/obj/build/bootstrap/debug/rustdoc" "-Wrustdoc::invalid_codeblock_attributes" "-Dwarnings" "-Znormalize-docs" "-Z" "unstable-options" "--test" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md" "--test-args" ""

stdout ----

running 1044 tests
---
test /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0790 (line 16933) ... ok

failures:

---- /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0045 (line 1030) stdout ----
error[E0277]: functions with the "rust-call" ABI must take a single non-self tuple argument
  |
  |
6 |     fn foo(x: u8, ...); // error!
  |        ^^^ the trait `Tuple` is not implemented for `u8`
error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
For more information about this error, try `rustc --explain E0277`.
Some expected error codes were not found: ["E0045"]
failures:
    /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0045 (line 1030)

test result: FAILED. 1009 passed; 1 failed; 34 ignored; 0 measured; 0 filtered out; finished in 8.95s

rust-log-analyzer avatar Oct 02 '22 21:10 rust-log-analyzer

:tada: Experiment pr-99943 is completed! :bar_chart: 110 regressed and 6 fixed (244726 total) :newspaper: Open the full report.

:warning: If you notice any spurious failure please add them to the blacklist! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot avatar Oct 03 '22 18:10 craterbot

:umbrella: The latest upstream changes (presumably #102627) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Oct 03 '22 20:10 bors

Seems like there are some (mis?)uses of Fn* implementations in the wild. I'm thinking if we should just make this change, as it should only be possible to hit with unstable features. The alternative, I think, would be to use a future-compat lint.

jackh726 avatar Oct 24 '22 00:10 jackh726

@bors r+ rollup=never

jackh726 avatar Nov 06 '22 17:11 jackh726

:pushpin: Commit ff8f84ccf6b208e41713da911333f20676472a48 has been approved by jackh726

It is now in the queue for this repository.

bors avatar Nov 06 '22 17:11 bors

:hourglass: Testing commit ff8f84ccf6b208e41713da911333f20676472a48 with merge 7eef946fc0e0eff40e588eab77b09b287accbec3...

bors avatar Nov 06 '22 17:11 bors

:sunny: Test successful - checks-actions Approved by: jackh726 Pushing 7eef946fc0e0eff40e588eab77b09b287accbec3 to master...

bors avatar Nov 06 '22 20:11 bors

Finished benchmarking commit (7eef946fc0e0eff40e588eab77b09b287accbec3): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.1% [-1.4%, -0.6%] 3
Improvements ✅
(secondary)
-3.0% [-4.0%, -0.4%] 7
All ❌✅ (primary) -1.1% [-1.4%, -0.6%] 3

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.5% [0.6%, 2.5%] 2
Regressions ❌
(secondary)
2.3% [2.2%, 2.3%] 2
Improvements ✅
(primary)
-1.0% [-1.6%, -0.5%] 2
Improvements ✅
(secondary)
-5.5% [-5.5%, -5.5%] 1
All ❌✅ (primary) 0.2% [-1.6%, 2.5%] 4

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.5% [0.5%, 0.5%] 1
Regressions ❌
(secondary)
4.6% [4.6%, 4.6%] 1
Improvements ✅
(primary)
-0.5% [-0.5%, -0.5%] 1
Improvements ✅
(secondary)
-2.5% [-2.5%, -2.5%] 1
All ❌✅ (primary) 0.0% [-0.5%, 0.5%] 2

rust-timer avatar Nov 06 '22 22:11 rust-timer