Suggest adding Result return type for associated method in E0277.
#126515
Conversation
rustbot
added
S-waiting-on-review
|
r? compiler |
| @@ -4598,19 +4598,41 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { | |||
| return; | |||
| } | |||
|
|
|||
| fn get_fn_decl<'tcx, 'hir>( | |||
There was a problem hiding this comment.
Could you rename this method? The first reaction I had seeing it was to suggest using node.fn_decl() and node.body_id(), but that's not what this function does: it chooses on which kind of items emit the diagnostic.
- Could you add a few comments explaining why only these cases? Why not foreign fns? Why not trait decls / trait impls.
There was a problem hiding this comment.
Could you rename this method? The first reaction I had seeing it was to suggest using
node.fn_decl()andnode.body_id(), but that's not what this function does: it chooses on which kind of items emit the diagnostic.
- Could you add a few comments explaining why only these cases? Why not foreign fns? Why not trait decls / trait impls.
Thank you. Sorry for the slow response.
Hello, this is introduced to solve issue # 125997, mainly suggestion for beginners.
It recommend adding a return type to the function definition. To prevent new errors, a corresponding return value needs to be added at the end of the function body.
For beginners, I hope to fix it directly here to avoid introducing new errors and requiring them to add return values by themselves.
I am concerned that the trait mehod may be used elsewhere, and modifying the function signature may result in errors in other trait impls. I think for foreign fn, we couldn't add a return value at the end of the function body.
There was a problem hiding this comment.
Please put summarized comments in the code. Having the explanation on github won't help the next person reading the code.
There was a problem hiding this comment.
Please put summarized comments in the code. Having the explanation on github won't help the next person reading the code.
Done. Thank you.
|
☔ The latest upstream changes (presumably #127493) made this pull request unmergeable. Please resolve the merge conflicts. |
cjgillot
added
S-waiting-on-author
|
I have added some comment. Please help me review again. Thank you. @cjgillot |
|
☔ The latest upstream changes (presumably #128041) made this pull request unmergeable. Please resolve the merge conflicts. |
|
R=me after rebase |
…ulacrum Move rustbook to its own workspace. This moves rustbook (the wrapper around mdbook) to its own Cargo workspace. This is done for two reasons: - Some users want to avoid having to check out documentation submodules if they are not working on documentation. These submodules are required for submodules that have Cargo dependencies in the tree (such as mdbook preprocessors). - The [pinned `memchr`](https://github.com/rust-lang/rust/blob/eb72697e41b00e5d8723f14c64a969d59d9b9474/compiler/rustc_ast/Cargo.toml#L10) is causing problems with updating. That pin is only necessary for the standard library, but unfortunately it is affecting all other crates. This will have some drawbacks: - A slight increase in the vendor directory size. My measurement shows about a 14M increase (0.7%), but somehow the compressed filesize is smaller. - The dependencies for rustbook now need to be managed separately. I have updated the cron job to try to mitigate this. - There will be a slight dist build time penalty. I'm not sure what it will be, since it heavily depends on the machine, but I suspect in the 30-45s range. - Adds more complexity to things like bootstrap and tidy. There are a few other alternatives considered: - Publish preprocessors on crates.io. This adds the burden of publishing every change, and ensuring those publishes happen and the sources don't get out of sync, and somehow syncing those updates back to rust-lang/rust during the automatic updates. This is also more work. - Move the submodules to subtrees. These have the added burden of doing updates in a way that is more difficult than submodules. I believe it also causes problems with GitHub's `#NNNN` tagging and closing issues. This is also more work. The only thing I haven't tested here is the cron job. However, there's a pretty decent chance this won't pass CI, or that I missed something.
rustbot
added
A-testsuite
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. |
|
There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged. You can start a rebase with the following commands: The following commits are merge commits: |
|
Sorry. I messed up this PR during rebase, I will resubmit. |
Suggest adding Result return type for associated method in E0277. Recommit rust-lang#126515 because I messed up during rebase, Suggest adding Result return type for associated method in E0277. For following: ```rust struct A; impl A { fn test4(&self) { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method } ``` Suggest: ```rust impl A { fn test4(&self) -> Result<(), Box<dyn std::error::Error>> { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method Ok(()) } } ``` For rust-lang#125997 r? `@cjgillot`
Suggest adding Result return type for associated method in E0277. Recommit rust-lang#126515 because I messed up during rebase, Suggest adding Result return type for associated method in E0277. For following: ```rust struct A; impl A { fn test4(&self) { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method } ``` Suggest: ```rust impl A { fn test4(&self) -> Result<(), Box<dyn std::error::Error>> { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method Ok(()) } } ``` For rust-lang#125997 r? ``@cjgillot``
Suggest adding Result return type for associated method in E0277. Recommit rust-lang#126515 because I messed up during rebase, Suggest adding Result return type for associated method in E0277. For following: ```rust struct A; impl A { fn test4(&self) { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method } ``` Suggest: ```rust impl A { fn test4(&self) -> Result<(), Box<dyn std::error::Error>> { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method Ok(()) } } ``` For rust-lang#125997 r? ```@cjgillot```
Suggest adding Result return type for associated method in E0277. Recommit rust-lang#126515 because I messed up during rebase, Suggest adding Result return type for associated method in E0277. For following: ```rust struct A; impl A { fn test4(&self) { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method } ``` Suggest: ```rust impl A { fn test4(&self) -> Result<(), Box<dyn std::error::Error>> { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method Ok(()) } } ``` For rust-lang#125997 r? ````@cjgillot````
Suggest adding Result return type for associated method in E0277. Recommit rust-lang#126515 because I messed up during rebase, Suggest adding Result return type for associated method in E0277. For following: ```rust struct A; impl A { fn test4(&self) { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method } ``` Suggest: ```rust impl A { fn test4(&self) -> Result<(), Box<dyn std::error::Error>> { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method Ok(()) } } ``` For rust-lang#125997 r? `@cjgillot`
Rollup merge of rust-lang#128084 - surechen:fix_125997_v1, r=cjgillot Suggest adding Result return type for associated method in E0277. Recommit rust-lang#126515 because I messed up during rebase, Suggest adding Result return type for associated method in E0277. For following: ```rust struct A; impl A { fn test4(&self) { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method } ``` Suggest: ```rust impl A { fn test4(&self) -> Result<(), Box<dyn std::error::Error>> { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method Ok(()) } } ``` For rust-lang#125997 r? `@cjgillot`
Suggest adding
Resultreturn type for associated method in E0277.For following:
Suggest:
For #125997