Fix error caused by recursion into Self. #10068
Conversation
Fixes rust-lang#10041. Prevent recusion into Self when it's a generic parameter. Added regression test from example in rust-lang#10041.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Alexendoo (or someone else) soon. Please see the contribution instructions for more information. |
rustbot
added
the
S-waiting-on-review
|
If this fix is not sufficient, I also created a revert with #10069 |
|
Let me know if I should remove the changelog line above since this is a fix. |
|
Hey @Nashenas88, this is worth a changelog entry. It would be good if it lists the lint the error was caused in and a rough example which code caused it. In this case, it might be: "... when a function returned a |
|
Note regarding the SegFault: It's not an actual memory issue but a stack overflow. When the failing test is loaded with and entering And looking at the backtrace with And they all center around the updated function: |
|
@xFrednet let me know if you'd like me to add any additional detail or change any wording here. |
|
Also, do I need to amend the commit with the changelog, or is it pulled from the github description instead? |
|
Sorry for the problem caused, but ideally I'd hope #9733 is not reverted because we have code that uses pattern in #7344. I did some poking and discovered that similar recursion can happen for assoc types as well, when using TAIT: #![feature(type_alias_impl_trait)]
type X = impl std::ops::Add<Output=X>;
struct Foo;
impl Foo {
fn new() -> X {
return 1;
}
}A more reliable solution might be to have a For a temporary solution, I suppose we can just get rid of recursion in |
|
Thank you for the updated and detailed description.
The entry is pulled from the GH description. Otherwise, it would also produce a bunch of merge conflicts.
It can take some time until PRs land on master. With this one, I hope to have it merged and back ported. This is very likely, and then we won't need a changelog entry. It's still better to have one in the description, in case it does take longer. |
|
Thanks for that @nbdd0121, I had been trying to find if there were a case like that but couldn't think of one |
|
I got this pr + the changes of #10068 (comment) together in branch and x.py test at least passes. 🎉 edit: #10081 |
See rust-lang#10068 (comment) for details -- original PR description of rust-lang#10068 -- fixes rust-lang#10041 Prevent recursion into Self when it's a generic parameter. Added regression test from example in rust-lang#10041. - \[x] Followed [lint naming conventions][lint_naming] - **N/A** - \[x] Added passing UI tests (including committed `.stderr` file) - \[x] `cargo test` passes locally - \[x] Executed `cargo dev update_lints` - **N/A** - \[x] Added lint documentation - **N/A** - \[x] Run `cargo dev fmt` changelog: [`new-ret-no-self`] Fix segmentation fault caused when generic parameter defaults to `Self` and is unspecified. For example, `fn uh_oh(&self) -> impl PartialOrd { ... }` has a hidden `Rhs=Self` as the generic parameter for `PartialOrd`.
Stack overflow fix See #10068 (comment) for details -- original PR description of #10068 -- fixes #10041 Prevent recursion into Self when it's a generic parameter. Added regression test from example in #10041. - \[x] Followed [lint naming conventions][lint_naming] - **N/A** - \[x] Added passing UI tests (including committed `.stderr` file) - \[x] `cargo test` passes locally - \[x] Executed `cargo dev update_lints` - **N/A** - \[x] Added lint documentation - **N/A** - \[x] Run `cargo dev fmt` changelog: [`new-ret-no-self`] Fix segmentation fault caused when generic parameter defaults to `Self` and is unspecified. For example, `fn uh_oh(&self)>
See rust-lang#10068 (comment) for details -- original PR description of rust-lang#10068 -- fixes rust-lang#10041 Prevent recursion into Self when it's a generic parameter. Added regression test from example in rust-lang#10041. - \[x] Followed [lint naming conventions][lint_naming] - **N/A** - \[x] Added passing UI tests (including committed `.stderr` file) - \[x] `cargo test` passes locally - \[x] Executed `cargo dev update_lints` - **N/A** - \[x] Added lint documentation - **N/A** - \[x] Run `cargo dev fmt` changelog: [`new-ret-no-self`] Fix segmentation fault caused when generic parameter defaults to `Self` and is unspecified. For example, `fn uh_oh(&self) -> impl PartialOrd { ... }` has a hidden `Rhs=Self` as the generic parameter for `PartialOrd`.
Stack overflow fix See #10068 (comment) for details -- original PR description of #10068 -- fixes #10041 Prevent recursion into Self when it's a generic parameter. Added regression test from example in #10041. - \[x] Followed [lint naming conventions][lint_naming] - **N/A** - \[x] Added passing UI tests (including committed `.stderr` file) - \[x] `cargo test` passes locally - \[x] Executed `cargo dev update_lints` - **N/A** - \[x] Added lint documentation - **N/A** - \[x] Run `cargo dev fmt` changelog: [`new-ret-no-self`] Fix segmentation fault caused when generic parameter defaults to `Self` and is unspecified. For example, `fn uh_oh(&self)>
|
@matthiaskrgr do we want to go with your PR then, and I'll close this one and #10069 ? |
|
my pr didn't work fully and I'm not sure how to fix it unfortunately 😅 |
|
I think we should go ahead and revert then, all the reasoning in https://forge.rust-lang.org/compiler/reviews.html#reverts applies here. |
Fix new_return_no_self with recursive bounds Fix #10041 This uses a hash set, as described in #10068 (comment) changelog: [`new_return_no_self`]: fix stack overflow when the return type is `impl Trait` and contains recursive bounds
|
☔ The latest upstream changes (presumably #10086) made this pull request unmergeable. Please resolve the merge conflicts. |
|
We just merged a forward-fix for this: #10086. |
fixes #10041
Prevent recursion into Self when it's a generic parameter. Added regression test from example in #10041.
.stderrfile)cargo testpasses locallycargo dev update_lints- N/Acargo dev fmtchangelog: [
new-ret-no-self] Fix segmentation fault caused when generic parameter defaults toSelfand is unspecified. For example,fn uh_oh(&self) -> impl PartialOrd { ... }has a hiddenRhs=Selfas the generic parameter forPartialOrd.