Skip to content

Suppresses RLC non-final field overwrite warning for safe constructor field initialization#7050

Open
iamsanjaymalakar wants to merge 137 commits into
typetools:masterfrom
iamsanjaymalakar:7049-dev
Open

Suppresses RLC non-final field overwrite warning for safe constructor field initialization#7050
iamsanjaymalakar wants to merge 137 commits into
typetools:masterfrom
iamsanjaymalakar:7049-dev

Conversation

@iamsanjaymalakar
Copy link
Copy Markdown
Member

@iamsanjaymalakar iamsanjaymalakar commented Apr 18, 2025

Summary by CodeRabbit

  • New Features

    • Enhanced resource leak checker to suppress false-positive warnings when a private field is assigned for the first time within a constructor.
  • Tests

    • Added comprehensive test suite covering various constructor initialization scenarios, including inline initializers, instance initializers, method calls, and conditional branching.

@iamsanjaymalakar iamsanjaymalakar self-assigned this Apr 18, 2025
Copy link
Copy Markdown
Contributor

@kelloggm kelloggm left a comment

Choose a reason for hiding this comment

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

Overall looks good, just a few minor comments. I'd like to take one more look before we merge, though.

@mernst mernst changed the title Suppresses RLC non-final field overwrite warning for safe constructor… #7049 Suppresses RLC non-final field overwrite warning for safe constructor field initialization Apr 18, 2025
@mernst
Copy link
Copy Markdown
Member

mernst commented Apr 18, 2025

@iamsanjaymalakar The typecheck job is not passing.

kelloggm
kelloggm previously approved these changes Jun 6, 2025
Copy link
Copy Markdown
Contributor

@kelloggm kelloggm left a comment

Choose a reason for hiding this comment

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

LGTM, I don't need to review again. Please address these comments and then request a review from Mike.

@mernst
Copy link
Copy Markdown
Member

mernst commented Jul 10, 2025

@iamsanjaymalakar Martin asked you to assign me when you had addressed his comments. Have you addressed them?

@iamsanjaymalakar iamsanjaymalakar assigned mernst and unassigned kelloggm Jul 14, 2025
@iamsanjaymalakar
Copy link
Copy Markdown
Member Author

@iamsanjaymalakar Martin asked you to assign me when you had addressed his comments. Have you addressed them?

Yes, I have. You can take a look.

Copy link
Copy Markdown
Member

@mernst mernst left a comment

Choose a reason for hiding this comment

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

Please clarify some documentation, and update the code if necessary. (I didn't read the code because there is no point in doing so until the documentation is clear.)

@mernst mernst assigned iamsanjaymalakar and unassigned mernst Jul 14, 2025
@mernst mernst removed their assignment Apr 25, 2026
@iamsanjaymalakar
Copy link
Copy Markdown
Member Author

@mernst CircleCI build is failing. I think it is not picking up the correct jdk annotations branch.

@mernst
Copy link
Copy Markdown
Member

mernst commented May 17, 2026

@iamsanjaymalakar I have fixed the CI problem.

@iamsanjaymalakar
Copy link
Copy Markdown
Member Author

@iamsanjaymalakar I have fixed the CI problem.

Thanks. It's ready for review then.

@mernst
Copy link
Copy Markdown
Member

mernst commented May 18, 2026

@iamsanjaymalakar Could you take a look at "ResourceLeakFirstInitConstructorTest" which is failing in CI? Thanks.

@iamsanjaymalakar
Copy link
Copy Markdown
Member Author

@iamsanjaymalakar Could you take a look at "ResourceLeakFirstInitConstructorTest" which is failing in CI? Thanks.

@mernst the test failure is happenning due to the same CI not picking up the latest code changes to the iamsanjaymalakar/jdk repo of 7049-dev branch. There in the latest commit I've added @SideEffectFree annotations to the FileInputStream's constructors.

In my local workspace, when I checkout to the typetools/jdks master branch I see the exact test failures in ResourceLeakFirstInitConstructorTest. And after switching to jdk's 7049-dev branch passes them. I'm seeing the exact same errors in the CI. So, I'm pretty confident that the new stubs not being picked up is the cause of the CI failures.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants