Make diagnostic for unsatisfied `Termination` bounds more precise #103142

fmease · 2022-10-17T10:14:12Z

Don't blindly emit a diagnostic claiming that “main has an invalid return type” if we encounter a type that should but doesn't implement std::process::Termination and isn't actually the return type of the program entry main.

Fixes #103052.

@rustbot label A-diagnostics T-compiler T-libs
r? diagnostics

rustbot · 2022-10-17T10:14:16Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

fmease · 2022-10-17T10:18:56Z

compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs

@@ -164,6 +164,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
            flags.push((sym::from_desugaring, Some(format!("{:?}", k))));
        }

+        if let ObligationCauseCode::MainFunctionType = obligation.cause.code() {


Of course, this branch could be generalized to pass along any obligation cause as a flag by using the hypothetical Debug impl for ObligationCauseCode similar to the from_desugaring branch above. However, ObligationCauseCode is quite a big & complex type and it doesn't impl Debug (yet). I am not really convinced that it's a good idea to derive an impl for it (if that's even possible).

Yea I'd think we'd want to add a method that just gives us the enum variant name without the fields if we wanted to expose all causes to users

oli-obk · 2022-10-18T09:30:39Z

@bors r+

cc @estebank you're gonna love this I think

bors · 2022-10-18T09:30:41Z

📌 Commit 684df4d has been approved by oli-obk

It is now in the queue for this repository.

Make diagnostic for unsatisfied `Termination` bounds more precise Don't blindly emit a diagnostic claiming that “*`main` has an invalid return type*” if we encounter a type that should but doesn't implement `std::process::Termination` and isn't actually the return type of the program entry `main`. Fixes rust-lang#103052. `@rustbot` label A-diagnostics T-compiler T-libs r? diagnostics

Rollup of 6 pull requests Successful merges: - rust-lang#103023 (Adding `fuchsia-ignore` and `needs-unwind` to compiler test cases) - rust-lang#103142 (Make diagnostic for unsatisfied `Termination` bounds more precise) - rust-lang#103154 (Fix typo in `ReverseSearcher` docs) - rust-lang#103159 (Remove the redundant `Some(try_opt!(..))` in `checked_pow`) - rust-lang#103163 (Remove all uses of array_assume_init) - rust-lang#103168 (Stabilize asm_sym) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

`#[test]`: Point at return type if `Termination` bound is unsatisfied Together with rust-lang#103142 (already merged) this fully fixes rust-lang#50291. I don't consider my current solution of changing a few spans “here and there” very clean since the failed obligation is a `FunctionArgumentObligation` and we point at a type instead of a function argument. If you agree with me on this point, I can offer to keep the spans of the existing nodes and instead inject `let _: AssertRetTyIsTermination<$ret_ty>;` (type to be defined in `libtest`) similar to `AssertParamIsEq` etc. used by some built-in derive-macros. I haven't tried that approach yet though and cannot promise that it would actually work out or be “cleaner” for that matter. `@rustbot` label A-libtest A-diagnostics r? `@estebank`

`#[test]`: Point at return type if `Termination` bound is unsatisfied Together with rust-lang#103142 (already merged) this fully fixes rust-lang#50291. I don't consider my current solution of changing a few spans “here and there” very clean since the failed obligation is a `FunctionArgumentObligation` and we point at a type instead of a function argument. If you agree with me on this point, I can offer to keep the spans of the existing nodes and instead inject `let _: AssertRetTyIsTermination<$ret_ty>;` (type to be defined in `libtest`) similar to `AssertParamIsEq` etc. used by some built-in derive-macros. I haven't tried that approach yet though and cannot promise that it would actually work out or be “cleaner” for that matter. ``@rustbot`` label A-libtest A-diagnostics r? ``@estebank``

`#[test]`: Point at return type if `Termination` bound is unsatisfied Together with rust-lang#103142 (already merged) this fully fixes rust-lang#50291. I don't consider my current solution of changing a few spans “here and there” very clean since the failed obligation is a `FunctionArgumentObligation` and we point at a type instead of a function argument. If you agree with me on this point, I can offer to keep the spans of the existing nodes and instead inject `let _: AssertRetTyIsTermination<$ret_ty>;` (type to be defined in `libtest`) similar to `AssertParamIsEq` etc. used by some built-in derive-macros. I haven't tried that approach yet though and cannot promise that it would actually work out or be “cleaner” for that matter. ```@rustbot``` label A-libtest A-diagnostics r? ```@estebank```

`#[test]`: Point at return type if `Termination` bound is unsatisfied Together with rust-lang#103142 (already merged) this fully fixes rust-lang#50291. I don't consider my current solution of changing a few spans “here and there” very clean since the failed obligation is a `FunctionArgumentObligation` and we point at a type instead of a function argument. If you agree with me on this point, I can offer to keep the spans of the existing nodes and instead inject `let _: AssertRetTyIsTermination<$ret_ty>;` (type to be defined in `libtest`) similar to `AssertParamIsEq` etc. used by some built-in derive-macros. I haven't tried that approach yet though and cannot promise that it would actually work out or be “cleaner” for that matter. ````@rustbot```` label A-libtest A-diagnostics r? ````@estebank````

Rollup of 6 pull requests Successful merges: - rust-lang#103023 (Adding `fuchsia-ignore` and `needs-unwind` to compiler test cases) - rust-lang#103142 (Make diagnostic for unsatisfied `Termination` bounds more precise) - rust-lang#103154 (Fix typo in `ReverseSearcher` docs) - rust-lang#103159 (Remove the redundant `Some(try_opt!(..))` in `checked_pow`) - rust-lang#103163 (Remove all uses of array_assume_init) - rust-lang#103168 (Stabilize asm_sym) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Make diagnostic for unsatisfied Termination bounds more precise

684df4d

rust-highfive assigned oli-obk Oct 17, 2022

rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 17, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 17, 2022

rustbot added the A-diagnostics Area: Messages for errors, warnings, and lints label Oct 17, 2022

fmease commented Oct 17, 2022

View reviewed changes

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 Oct 18, 2022

Dylan-DPC mentioned this pull request Oct 18, 2022

Rollup of 6 pull requests #103184

Closed

JohnTitor mentioned this pull request Oct 18, 2022

Rollup of 6 pull requests #103188

Merged

bors merged commit 472a874 into rust-lang:master Oct 18, 2022

rustbot added this to the 1.66.0 milestone Oct 18, 2022

fmease deleted the fix-103052 branch October 19, 2022 16:08

fmease mentioned this pull request Oct 19, 2022

error message when unit test type does not implement Termination is ungreat #50291

Closed

fmease mentioned this pull request Oct 23, 2022

#[test]: Point at return type if Termination bound is unsatisfied #103445

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make diagnostic for unsatisfied `Termination` bounds more precise #103142

Make diagnostic for unsatisfied `Termination` bounds more precise #103142

fmease commented Oct 17, 2022 •

edited

Loading

rustbot commented Oct 17, 2022

fmease Oct 17, 2022 •

edited

Loading

oli-obk Oct 18, 2022

oli-obk commented Oct 18, 2022

bors commented Oct 18, 2022

Make diagnostic for unsatisfied Termination bounds more precise #103142

Make diagnostic for unsatisfied Termination bounds more precise #103142

Conversation

fmease commented Oct 17, 2022 • edited Loading

rustbot commented Oct 17, 2022

fmease Oct 17, 2022 • edited Loading

Choose a reason for hiding this comment

oli-obk Oct 18, 2022

Choose a reason for hiding this comment

oli-obk commented Oct 18, 2022

bors commented Oct 18, 2022

Make diagnostic for unsatisfied `Termination` bounds more precise #103142

Make diagnostic for unsatisfied `Termination` bounds more precise #103142

fmease commented Oct 17, 2022 •

edited

Loading

fmease Oct 17, 2022 •

edited

Loading