Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignoring empty statements in closures. #6128

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rscprof
Copy link

@rscprof rscprof commented Mar 29, 2024

Solve #6116

Copy link
Contributor

@ytmimi ytmimi left a comment

Choose a reason for hiding this comment

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

Thanks for another PR. I think we can simplify the implementation of iter_stmts_without_empty slightly, and I'd like to expand on the test cases that you've already added.

src/closures.rs Outdated Show resolved Hide resolved
tests/source/issue-6116/main.rs Show resolved Hide resolved
Copy link
Contributor

@ytmimi ytmimi left a comment

Choose a reason for hiding this comment

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

Thanks for adding those additional test cases, and for applying the other feedback. Please take a look at my follow up comments when you have a moment.

src/closures.rs Outdated Show resolved Hide resolved
src/closures.rs Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this test case is a great addition! Also wanted to note that you can totally continue to add new files as test cases, though you might find it easier to place all of these test cases into a single file.

Copy link
Author

Choose a reason for hiding this comment

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

I don't know what I have to do)))

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm letting you know that instead of creating multiple test files you can add more than one test case to a single file.

for example:

fn foo() -> fn(i32) -> i32 {
    |a| {
        ;
        a
    }
}

fn bar() -> fn(i32) -> i32 {
    |a| {
        ;
        a;
        b
    }
}

fn foobar() -> fn(i32) -> i32 {
    |a| {
        ;
	;
	;;;;
        a
    }
}

fn baz() -> fn(i32) -> i32 {
    |a| {
        /*comment before empty statement */;
        a
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants