Fix suggestions when returning a bare trait from an async fn. by hirschenberger · Pull Request #131882 · rust-lang/rust

hirschenberger · 2024-10-18T09:13:42Z

Don't assume we always return a trait object and suggest the dyn keyword. Suggest adding impl instead.

rustbot · 2024-10-18T09:13:50Z

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-10-18T09:13:52Z

HIR ty lowering was modified

cc @fmease

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

tests/ui/traits/suggest-impl-on-bare-trait-return.rs

Don't assume we always return a trait object and suggest the dyn keyword. Suggest adding impl instead.

rust-log-analyzer · 2024-10-18T10:00:12Z

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

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

------
 > importing cache manifest from ghcr.io/rust-lang/rust-ci-cache:c32c805632780b5c1de330e3f44561b336c2efe163bc0990acb392390157a8e1d9f855d75914a239aa40c49d77f4a837247d05d2f8d46f554b98e1f46712a3e3:
------
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-18]
debug: `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` configured.
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-18', '--enable-llvm-link-shared', '--set', 'rust.randomize-layout=true', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-18/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.randomize-layout := True
configure: rust.thin-lto-import-instr-limit := 10
---
+   --> $DIR/suggest-impl-on-bare-trait-return.rs:4:19
3    |
- LL | fn fun() -> Trait {
-    |             ^^^^^
+ LL | async fn fun() -> Trait {
6    |
7 help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
8    |

---
11 help: alternatively, you can return an owned trait object
12    |
- LL | fn fun() -> Box<dyn Trait> {
-    |             +++++++      +
+ LL | Box<dyn async fn fun() -> Trait> {
+    | +++++++                        +
+ help: you might have meant to write a bound here
+    |
+ LL | : Trait {
+    | ~
16 error: aborting due to 1 previous error
17 



The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/traits/suggest-impl-on-bare-trait-return/suggest-impl-on-bare-trait-return.stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args traits/suggest-impl-on-bare-trait-return.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/traits/suggest-impl-on-bare-trait-return.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/traits/suggest-impl-on-bare-trait-return" "-A" "unused" "-A" "internal_features" "-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/ui/traits/suggest-impl-on-bare-trait-return/auxiliary" "--edition=2021"
--- stderr -------------------------------
error[E0782]: expected a type, found a trait
##[error]  --> /checkout/tests/ui/traits/suggest-impl-on-bare-trait-return.rs:4:19
   |
   |
LL | async fn fun() -> Trait {  //~ ERROR expected a type, found a trait
   |
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
   |
   |
LL | async fn fun() -> impl Trait {  //~ ERROR expected a type, found a trait
help: alternatively, you can return an owned trait object
   |
   |
LL | Box<dyn async fn fun() -> Trait> {  //~ ERROR expected a type, found a trait
   | +++++++                        +
help: you might have meant to write a bound here
   |
LL | : Trait {  //~ ERROR expected a type, found a trait
   | ~
error: aborting due to 1 previous error

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

compiler-errors · 2024-10-18T13:36:10Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

+    /// returns the `i32`.
+    ///
+    /// [`OpaqueDef`]: hir::TyKind::OpaqueDef
+    pub fn get_future_inner_return_ty<'a>(hir_ty: &'a hir::Ty<'a>) -> Option<&'a hir::Ty<'a>> {


Just make this an inherent method please. There's no use of &self. Also rename the 'a to 'tcx, as it is more consistent.

This function/method was moved from Mir to Hir. I thought compiler stages should only have access to previous stages, not following ones, right? I hope this is ok.

I saw that in the definition of get_future_inner_return_ty in Mir, self was not used so I made it a non-inherent (static?) method to provide access to the one place in Mir where it is used.

Sorry, typo, I mean make it a free function. There's no reason to put this into an impl :)

compiler-errors · 2024-10-18T13:37:49Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

            tcx.parent_hir_node(self_ty.hir_id),
            hir::Node::Ty(hir::Ty { kind: hir::TyKind::Ref(..), .. })
        );
+        let is_non_trait_object = |ty: &'tcx hir::Ty<'_>| {


This feels unfortunately somewhat ad-hoc, since any place we want to inspect the hir::TyKind, like for any diagnostics, we'll need to call the get_future_inner_return_ty.

hirschenberger · 2024-10-18T13:58:12Z

@compiler-errors @fmease

I'm working on this, as some cases are not correct yet. So feel free to delay reviews, although I appreciate your feedback.

bors · 2024-10-31T01:35:17Z

☔ The latest upstream changes (presumably #132371) made this pull request unmergeable. Please resolve the merge conflicts.

Dylan-DPC · 2025-03-02T06:54:55Z

@hirschenberger any updates on this? thanks

hirschenberger · 2025-03-04T15:11:26Z

@hirschenberger any updates on this? thanks
Sorry, no progress yet.

If it's urgent, feel free to reassign.

I hope I find some time to fix this in some weeks.

Dylan-DPC · 2025-07-31T14:37:12Z

Closing this as it has been inactive for a long time. I think it's better to do it fresh when you have the time for it. Thanks for contributing.

rustbot assigned davidtwco Oct 18, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 18, 2024

fmease reviewed Oct 18, 2024

View reviewed changes

tests/ui/traits/suggest-impl-on-bare-trait-return.rs Outdated Show resolved Hide resolved

Fix suggestions when returning a bare trait from an async fn.

5642733

Don't assume we always return a trait object and suggest the dyn keyword. Suggest adding impl instead.

hirschenberger force-pushed the issue131661 branch from d63cc44 to 5642733 Compare October 18, 2024 09:30

hirschenberger marked this pull request as draft October 18, 2024 10:02

compiler-errors reviewed Oct 18, 2024

View reviewed changes

jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2025

Dylan-DPC closed this Jul 31, 2025

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 31, 2025

Uh oh!

Conversation

hirschenberger commented Oct 18, 2024

Uh oh!

rustbot commented Oct 18, 2024

Uh oh!

rustbot commented Oct 18, 2024

Uh oh!

Uh oh!

rust-log-analyzer commented Oct 18, 2024

Uh oh!

compiler-errors Oct 18, 2024

Choose a reason for hiding this comment

Uh oh!

hirschenberger Oct 18, 2024

Choose a reason for hiding this comment

Uh oh!

compiler-errors Oct 18, 2024

Choose a reason for hiding this comment

Uh oh!

compiler-errors Oct 18, 2024

Choose a reason for hiding this comment

Uh oh!

hirschenberger commented Oct 18, 2024

Uh oh!

bors commented Oct 31, 2024

Uh oh!

Dylan-DPC commented Mar 2, 2025

Uh oh!

hirschenberger commented Mar 4, 2025

Uh oh!

Dylan-DPC commented Jul 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants