Skip to content

nightly warning: unused ControlFlow that must be used #7736

New issue

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

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

Already on GitHub? Sign in to your account

Closed
BenWiederhake opened this issue Apr 12, 2025 · 3 comments
Closed

nightly warning: unused ControlFlow that must be used #7736

BenWiederhake opened this issue Apr 12, 2025 · 3 comments

Comments

@BenWiederhake
Copy link
Collaborator

When compiling with nightly (cargo 1.88.0-nightly 2025-03-26 or newer, probably older as well), we get some compilation warnings:

warning: unused `ControlFlow` that must be used
   --> src/uucore/src/lib/features/format/mod.rs:281:9
    |
281 |         item?.write(&mut writer, &mut args)?;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_must_use)]` on by default
help: use `let _ = ...` to ignore the resulting value
    |
281 |         let _ = item?.write(&mut writer, &mut args)?;
    |         +++++++

warning: unused `ControlFlow` that must be used
   --> src/uu/echo/src/echo.rs:153:17
    |
153 | /                 match item {
154 | |                     EscapedChar::End => return Ok(()),
155 | |                     c => c.write(&mut *stdout_lock)?,
156 | |                 };
    | |_________________^
    |
    = note: `#[warn(unused_must_use)]` on by default
help: use `let _ = ...` to ignore the resulting value
    |
153 |                 let _ = match item {
    |                 +++++++

It seems that there have been some changes to the inner workings of the compiler: rust-lang/rust#137449

I'm not quite sure how to deal with this, but it seems that this warning will make it's way into stable eventually I guess, so let's "fix" this issue? However, in both cases I don't fully understand why that is even necessary, so I don't trust the compiler-suggested code.

@tertsdiepraam
Copy link
Member

tertsdiepraam commented Apr 14, 2025

The ControlFlow dictated whether the formatting should exit early, which happens if \0 was passed I think. Both of these cases might need to return in that case, but we'd have to test to see what GNU does.

Edit: It's \c, not \0

@alexs-sh
Copy link
Contributor

alexs-sh commented May 4, 2025

Hello @BenWiederhake, @tertsdiepraam

Thank you for your observations and comments.

I’ve opened #7881 PR with some initial changes for review, discussion, and testing on real examples. While experimenting, I noticed the following:

  1. The most straightforward way to trigger ControlFlow::Break is by using EscapedCharacter::End (see here). This seems like an appropriate way to exit early in this case.

  2. To me, switching from analyzing the item type to analyzing the return value in echo feels like a simplification. We avoid dealing with the item’s internal details and instead rely on a common return value.

  3. The printf utility already relies on ControlFlow, so using the same approach in echo seems like a good idea. Possibly, I’ll need to update the style of handling the values to align with the printf approach. However, let's first review and approve or decline the general approach before focusing on stylistic details.

@BenWiederhake
Copy link
Collaborator Author

Closed by #7881. Awesome, thanks!

@alexs-sh Did you know? If you write something like "Fixes: #7736" in the PR description (or, if applicable, in the body of the relevant commit), then GitHub auto-closes the linked issue once the PR is merged. If you don't feel comfortable with it, that's of course fine, too. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants