Skip to content

uucore/echo:handle ControlFlow result #7881

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

Merged
merged 3 commits into from
May 4, 2025

Conversation

alexs-sh
Copy link
Contributor

@alexs-sh alexs-sh commented May 4, 2025

About

Issue 7736

Hello Everyone,

This PR introduces analysis of returned ControlFlow values, helping to eliminate compiler warnings about unused return values on nightly toolchains. It also adds the ability to exit early when encountering EscapedCharacter::End, which seems like the correct behavior in that case. [1]

While working on this, I noticed that the printf utility already relies on ControlFlow results. So this change also helps unify the approaches and code between printf and echo.

Please note: these are initial changes, intended for review, discussion, and testing on real examples. Any feedback or observations are highly appreciated.

Thank you

[1] Output of GNU Coreutils (9.7) and uutils/coreutils

GNU coreutils

/usr/bin/echo -e "abc\c123"
abc

uutils/coreutils

./target/debug/echo -e "abc\c123"
abc

alexs-sh added 2 commits May 4, 2025 12:18
This commit explicitly handles the result of the `write` function and
allows an early stop when a `ControlFlow::Break` is received.

It seems that currently only writing `EscapedChar::End` will trigger a
break. In all other cases, `ControlFlow::Continue` is returned.
This commit analyzes the return value of the `write` function and stops
the echo when a break is received. It allows:

- simplifying the function's logic. Now, we don't rely on the item type
  and only analyze the return value.

- avoiding compiler warnings about the unused `ControlFlow` result
  (nightly only).

Since a break can only be triggered by `EscapedChar::End`, the old and new
versions should be equivalent.
Copy link

github-actions bot commented May 4, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

Copy link
Member

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! I confirmed that GNU echo indeed has this behaviour. Could you add a test for it?

@alexs-sh
Copy link
Contributor Author

alexs-sh commented May 4, 2025

@tertsdiepraam Thanks. I’ve added a test for the Ctrl-C sequence

Copy link

github-actions bot commented May 4, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

Copy link
Member

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks!

@tertsdiepraam tertsdiepraam merged commit 7d5cfbc into uutils:main May 4, 2025
69 of 70 checks passed
@alexs-sh alexs-sh deleted the 7736-control-flow-experiments branch May 11, 2025 19:29
RenjiSann pushed a commit to RenjiSann/coreutils that referenced this pull request May 23, 2025
…ments

uucore/echo:handle ControlFlow result
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

Successfully merging this pull request may close these issues.

2 participants