Add lint for recursive default impls by mj10021 · Pull Request #128737 · rust-lang/rust

mj10021 · 2024-08-06T15:16:38Z

Fixes #128421, where it was pointed out that recursion in default impls can be confusing and always results in failure.

rustbot · 2024-08-06T15:16:46Z

rustbot has assigned @chenyukang.
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

compiler-errors

I don't think this should be a new lint. We already have a lint for when we detect recursive calls, and we can change the wording for that lint if we detect that it's a recursive Default invocation.

compiler-errors · 2024-08-06T17:31:56Z

compiler/rustc_lint_defs/src/builtin.rs

+    /// default struct for this field. This will always lead to an infinite loop
+    /// so it is denied.
+    pub RECURSIVE_DEFAULT_IMPL,
+    Deny,


This needs T-lang approval if it's going to be a Deny lint.

mj10021 · 2024-08-06T19:56:22Z

I don't think this should be a new lint. We already have a lint for when we detect recursive calls, and we can change the wording for that lint if we detect that it's a recursive Default invocation.

Do you think that it makes sense for the Self { ..Default::default() } case to be denied since it is a guaranteed infinite loop or it is too specific off a case to warrant special treatment?

mj10021 · 2024-08-06T20:05:40Z

@rustbot label +T-Lang

chenyukang · 2024-08-24T02:09:51Z

yeah, I think it's better to change the wording for existing lint for this specific scenario.

chenyukang · 2024-08-24T02:19:25Z

@rustbot author

bors · 2024-09-23T05:06:08Z

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

alex-semenyuk · 2024-10-30T12:53:57Z

@mj10021
From wg-triage. Do you have update on this PR?

mj10021 · 2024-10-31T02:07:57Z

@mj10021 From wg-triage. Do you have update on this PR?

Oops, this got away from me, will update the PR shortly

mj10021 · 2024-11-05T14:42:48Z

Does this look more like the type of note that was in mind?

compiler/rustc_mir_build/messages.ftl

compiler/rustc_mir_build/src/errors.rs

compiler/rustc_mir_build/src/lints.rs

compiler/rustc_mir_build/messages.ftl

compiler/rustc_mir_build/src/lints.rs

mj10021 · 2024-11-24T18:31:40Z

@rustbot review

compiler-errors

I don't think that this change carries enough benefit, especially b/c I don't believe we can actually tell we're actually in a FRU to make this note accurate.

compiler-errors · 2024-12-16T19:29:48Z

tests/ui/lint/lint-unconditional-recursion.stderr

   |                 ------------------ recursive call site
   |
   = help: a `loop` may express intention better if this is on purpose
+   = help: calling `..Default::default()` in a `Default` implementation leads to infinite recursion, and does not initialize the fields of a struct or enum


This is evidence of false positives for this lint.

compiler-errors · 2024-12-16T19:30:53Z

r? compiler

bors · 2024-12-18T19:39:00Z

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

Dylan-DPC · 2025-02-14T15:18:17Z

@mj10021 any updates on this pr? thanks

mj10021 · 2025-03-03T14:27:59Z

@mj10021 any updates on this pr? thanks

Sorry, I thought the PR was rejected, are these changes still wanted?

Dylan-DPC · 2025-03-30T04:54:50Z

@mj10021 ah i was waiting to see if maybe there is some solution to this. I guess not, so I'm fine with closing it. Thanks for contributing.

rustbot assigned chenyukang Aug 6, 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 Aug 6, 2024

This comment has been minimized.

Sign in to view

compiler-errors suggested changes Aug 6, 2024

View reviewed changes

compiler-errors reviewed Aug 6, 2024

View reviewed changes

rustbot added the T-lang Relevant to the language team label Aug 6, 2024

rustbot 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 Aug 24, 2024

compiler-errors self-assigned this Aug 24, 2024

mj10021 force-pushed the issue-128421-fix branch from 9547a7f to 9c0ccb3 Compare November 5, 2024 14:42

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

compiler-errors suggested changes Nov 21, 2024

View reviewed changes

compiler/rustc_mir_build/messages.ftl Outdated Show resolved Hide resolved

compiler/rustc_mir_build/src/errors.rs Outdated Show resolved Hide resolved

compiler/rustc_mir_build/src/lints.rs Outdated Show resolved Hide resolved

rustbot 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 Nov 21, 2024

add check and note for recursive default impl

1251b2b

mj10021 force-pushed the issue-128421-fix branch from 9c0ccb3 to 8f7ca38 Compare November 21, 2024 19:26

compiler-errors suggested changes Nov 22, 2024

View reviewed changes

compiler/rustc_mir_build/messages.ftl Outdated Show resolved Hide resolved

compiler/rustc_mir_build/src/lints.rs Outdated Show resolved Hide resolved

mj10021 force-pushed the issue-128421-fix branch from 8f7ca38 to 23ee908 Compare November 22, 2024 00:49

apply comments

d9ee420

mj10021 force-pushed the issue-128421-fix branch from 23ee908 to d9ee420 Compare November 22, 2024 02:00

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

compiler-errors suggested changes Dec 16, 2024

View reviewed changes

rustbot 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 Dec 16, 2024

rustbot assigned wesleywiser and unassigned chenyukang and compiler-errors Dec 16, 2024

Dylan-DPC closed this Mar 30, 2025

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

Uh oh!

Conversation

mj10021 commented Aug 6, 2024

Uh oh!

rustbot commented Aug 6, 2024

Uh oh!

This comment has been minimized.

This comment has been minimized.

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

compiler-errors Aug 6, 2024

Choose a reason for hiding this comment

Uh oh!

mj10021 commented Aug 6, 2024

Uh oh!

mj10021 commented Aug 6, 2024

Uh oh!

chenyukang commented Aug 24, 2024

Uh oh!

chenyukang commented Aug 24, 2024

Uh oh!

bors commented Sep 23, 2024

Uh oh!

alex-semenyuk commented Oct 30, 2024

Uh oh!

mj10021 commented Oct 31, 2024

Uh oh!

mj10021 commented Nov 5, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mj10021 commented Nov 24, 2024

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

compiler-errors Dec 16, 2024

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Dec 16, 2024

Uh oh!

bors commented Dec 18, 2024

Uh oh!

Dylan-DPC commented Feb 14, 2025

Uh oh!

mj10021 commented Mar 3, 2025

Uh oh!

Dylan-DPC commented Mar 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants