Suppresses RLC non-final field overwrite warning for safe constructor field initialization#7050
Suppresses RLC non-final field overwrite warning for safe constructor field initialization#7050iamsanjaymalakar wants to merge 122 commits intotypetools:masterfrom
Conversation
… field initialization
kelloggm
left a comment
There was a problem hiding this comment.
Overall looks good, just a few minor comments. I'd like to take one more look before we merge, though.
|
@iamsanjaymalakar The typecheck job is not passing. |
kelloggm
left a comment
There was a problem hiding this comment.
LGTM, I don't need to review again. Please address these comments and then request a review from Mike.
|
@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. |
mernst
left a comment
There was a problem hiding this comment.
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.)
|
|
||
| public FirstAssignmentInConditional(boolean b) { | ||
| try { | ||
| if (b) { |
There was a problem hiding this comment.
I don't see how it would turn into a CFG dependency.
| try { | ||
| if (b) { | ||
| // ::error: (required.method.not.called) | ||
| s = new FileInputStream("test1.txt"); // false positive: first write in this branch |
There was a problem hiding this comment.
I don't see that handling branches requires control-flow reasoning or a control-flow graph. It can be done purely (and simply) in the AST. If you don't want to do it, that is one thing, but please provide the correct rationale.
I think the wording you are looking for is "lexically first"; "AST-only" does not convey anything to me.
|
@iamsanjaymalakar When you address a code review comment, please mark it as "resolved" so that we know its status and it does not clutter this code review webpage. |
…ork into 7049-dev
Code review changes
Summary by CodeRabbit
New Features
Tests