-
Notifications
You must be signed in to change notification settings - Fork 348
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
Optimize temporary variable creation by excluding void method calls; … #6569
Conversation
if (node instanceof MethodInvocationNode) { | ||
MethodInvocationTree methodInvocationTree = (MethodInvocationTree) node.getTree(); | ||
ExecutableElement executableElement = TreeUtils.elementFromUse(methodInvocationTree); | ||
if (ElementUtils.getType(executableElement).getKind() == TypeKind.VOID) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you test that the type of the element is void in these cases? I would have thought we would need to write some code to get the return type and check that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't added any new test case to check that. Checked using the existing test cases. I'll add a new test case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I looked at this similar code and it is doing the same check so this is fine.
Adding a test for this directly will be hard since it would depend on the temporary variable names or something like that. I don't think it's necessary as long as extant tests are passing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like CI is failing due to formatting: https://dev.azure.com/checkerframework/checkerframework/_build/results?buildId=15291&view=logs&j=c326031a-fc8d-5e8e-5aaf-1127ae89918f&t=56a01ac5-ee6c-5ec2-e15f-d6c913c7703c&l=334 otherwise this change LGTM
…fixes #6568.