Skip to content

Accommodate yield points in the format_args expansion #93461

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Merged
merged 3 commits into from
Jan 31, 2022
Merged

Accommodate yield points in the format_args expansion #93461

merged 3 commits into from
Jan 31, 2022

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Jan 29, 2022

Fixes #93274.

For the case println!("{} {:?}", "", async {}.await) in the issue, the expansion before:

::std::io::_print(
    ::core::fmt::Arguments::new_v1(
        &["", " ", "\n"],
        &[
            ::core::fmt::ArgumentV1::new(&"", ::core::fmt::Display::fmt),
            ::core::fmt::ArgumentV1::new(&async {}.await, ::core::fmt::Debug::fmt),
        ],
    ),
);

After:

::std::io::_print(
    ::core::fmt::Arguments::new_v1(
        &["", " ", "\n"],
        &match (&"", &async {}.await) {
            _args => [
                ::core::fmt::ArgumentV1::new(_args.0, ::core::fmt::Display::fmt),
                ::core::fmt::ArgumentV1::new(_args.1, ::core::fmt::Debug::fmt),
            ],
        },
    ),
);

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 29, 2022
@rust-highfive
Copy link
Contributor

r? @davidtwco

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 29, 2022
@rust-log-analyzer

This comment has been minimized.

# to view
@dtolnay

This comment has been minimized.

# to view
@cjgillot

This comment has been minimized.

# to view
@rust-log-analyzer

This comment has been minimized.

# to view
danielhenrymantilla
danielhenrymantilla approved these changes Jan 30, 2022
View reviewed changes
@dtolnay
Add regression test for issue 93274
1dd02e3 
Currently fails with:

    error: future cannot be sent between threads safely
      --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:21:17
       |
    LL |     assert_send(with_await());
       |                 ^^^^^^^^^^^^ future returned by `with_await` is not `Send`
       |
       = help: the trait `Sync` is not implemented for `core::fmt::Opaque`
    note: future is not `Send` as this value is used across an await
      --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:11:37
       |
    LL |     println!("{} {:?}", "", async {}.await);
       |     --------------------------------^^^^^^-
       |     |                               |
       |     |                               await occurs here, with `$crate::format_args_nl!($($arg)*)` maybe used later
       |     has type `ArgumentV1<'_>` which is not `Send`
       |     `$crate::format_args_nl!($($arg)*)` is later dropped here
    note: required by a bound in `assert_send`
      --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:18:24
       |
    LL | fn assert_send(_: impl Send) {}
       |                        ^^^^ required by this bound in `assert_send`

    error: future cannot be sent between threads safely
      --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:22:17
       |
    LL |     assert_send(with_macro_call());
       |                 ^^^^^^^^^^^^^^^^^ future returned by `with_macro_call` is not `Send`
       |
       = help: the trait `Sync` is not implemented for `core::fmt::Opaque`
    note: future is not `Send` as this value is used across an await
      --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:6:17
       |
    LL |         async {}.await
       |                 ^^^^^^ await occurs here, with `$crate::format_args_nl!($($arg)*)` maybe used later
    ...
    LL |     println!("{} {:?}", "", m!());
       |     -----------------------------
       |     |                       |
       |     |                       in this macro invocation
       |     has type `ArgumentV1<'_>` which is not `Send`
       |     `$crate::format_args_nl!($($arg)*)` is later dropped here
    note: required by a bound in `assert_send`
      --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:18:24
       |
    LL | fn assert_send(_: impl Send) {}
       |                        ^^^^ required by this bound in `assert_send`
       = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)

    error: aborting due to 2 previous errors
@dtolnay
Accommodate yield points in the format_args expansion
47f92a5
@dtolnay
Mac calls
858d6a0
@dtolnay
Copy link
Member Author

dtolnay commented Jan 30, 2022

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 31, 2022

📌 Commit 858d6a0 has been approved by davidtwco

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2022
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 31, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2022
@bors
Auto merge of rust-lang#93499 - matthiaskrgr:rollup-icdex11, r=matthi…
415c9f9 
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#93395 (Improve suggestion for escaping reserved keywords)
 - rust-lang#93403 (review the total_cmp documentation)
 - rust-lang#93461 (Accommodate yield points in the format_args expansion)
 - rust-lang#93462 (Document `SystemTime` platform precision)
 - rust-lang#93471 (unix: Use metadata for `DirEntry::file_type` fallback)
 - rust-lang#93480 (Remove deprecated and unstable slice_partition_at_index functions)
 - rust-lang#93485 (core: Remove some redundant {}s from the sorting code)
 - rust-lang#93494 (kmc-solid: Inherit the calling task's base priority in `Thread::new`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c1e2948 into rust-lang:master Jan 31, 2022
@rustbot rustbot added this to the 1.60.0 milestone Jan 31, 2022
@dtolnay dtolnay deleted the fmtyield branch January 31, 2022 17:54
@dtolnay dtolnay added A-fmt Area: `core::fmt` and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 14, 2025
@m-ou-se m-ou-se mentioned this pull request Apr 11, 2025
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@davidtwco davidtwco davidtwco approved these changes

@danielhenrymantilla danielhenrymantilla danielhenrymantilla approved these changes

Assignees

@davidtwco davidtwco

Labels
A-fmt Area: `core::fmt` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.60.0
Development

Successfully merging this pull request may close these issues.

Regression in async generator and fmt internals: loss of Send
8 participants
@dtolnay @rust-highfive @rust-log-analyzer @cjgillot @davidtwco @bors @danielhenrymantilla @rustbot