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

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

@@ -223,10 +223,17 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
            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.

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

hirschenberger commented Oct 18, 2024

rustbot commented Oct 18, 2024

rustbot commented Oct 18, 2024

rust-log-analyzer commented Oct 18, 2024

compiler-errors Oct 18, 2024

hirschenberger Oct 18, 2024

compiler-errors Oct 18, 2024

compiler-errors Oct 18, 2024

hirschenberger commented Oct 18, 2024

bors commented Oct 31, 2024

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

Are you sure you want to change the base?

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

Conversation

hirschenberger commented Oct 18, 2024

rustbot commented Oct 18, 2024

rustbot commented Oct 18, 2024

rust-log-analyzer commented Oct 18, 2024

compiler-errors Oct 18, 2024

Choose a reason for hiding this comment

hirschenberger Oct 18, 2024

Choose a reason for hiding this comment

compiler-errors Oct 18, 2024

Choose a reason for hiding this comment

compiler-errors Oct 18, 2024

Choose a reason for hiding this comment

hirschenberger commented Oct 18, 2024

bors commented Oct 31, 2024