Skip to content

Conversation

GuillaumeGomez
Copy link
Member

Follow-up of #11938.

r? @llogiq

changelog: Extend UNCONDITIONAL_RECURSION to check for ToString implementations

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 18, 2023
@GuillaumeGomez GuillaumeGomez force-pushed the UNCONDITIONAL_RECURSION-tostring branch from 381531e to efddb33 Compare December 18, 2023 15:58
impl std::string::ToString for S6 {
fn to_string(&self) -> String {
//~^ ERROR: function cannot return without recursing
self.to_string()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps add another test where self is put into a local before calling its to_string?

And another one calling (self as &Self).to_string() just for extra fun? 😋

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps add another test where self is put into a local before calling its to_string?

For that one, we agree that it's just for cases like:

let x = self;
x.to_string()

and nothing else? Because otherwise it becomes quite tricky to check if self was mutated... However if you know of a way to check it, I'm very interested. :)

And another one calling (self as &Self).to_string() just for extra fun? 😋

Sure. ^^'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However if you know of a way to check it, I'm very interested. :)

There's the clippy_utils::expr_or_init function, which should be helpful for that.

@rustbot author

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 25, 2023
@GuillaumeGomez GuillaumeGomez force-pushed the UNCONDITIONAL_RECURSION-tostring branch from efddb33 to d161f3b Compare December 30, 2023 16:12
@GuillaumeGomez
Copy link
Member Author

Updated and added the suggested tests. :)

@llogiq
Copy link
Contributor

llogiq commented Dec 31, 2023

Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Dec 31, 2023

📌 Commit d161f3b has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 31, 2023

⌛ Testing commit d161f3b with merge 44c99a8...

@bors
Copy link
Contributor

bors commented Dec 31, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 44c99a8 to master...

@bors bors merged commit 44c99a8 into rust-lang:master Dec 31, 2023
@GuillaumeGomez GuillaumeGomez deleted the UNCONDITIONAL_RECURSION-tostring branch December 31, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants