Remove unused parameters, which fixes a warning suppression #761
Conversation
|
@wmdietl Looks like time out limit 110s is still not enough for some cases. Can you trigger the nonjunit_jdk8 to rerun? |
| * @param backup value to use if no most specific value is found | ||
| */ | ||
| @SuppressWarnings("UnusedVariable") // TODO clean this up | ||
| public MostSpecificVisitor(TypeMirror aTypeMirror, TypeMirror bTypeMirror, V backup) { | ||
| public MostSpecificVisitor(V backup) { |
There was a problem hiding this comment.
Thanks for the cleanup!
Above, if (ms.error) { return backup; }, is backup always null if ms.error is true?
A few nullness annotations also seem to be missing, e.g. on backupAMSet.
There was a problem hiding this comment.
is backup always null if ms.error is true?
I think no, the visitor will not reassign the value of backup. It only depends on the input.
A few nullness annotations also seem to be missing, e.g. on backupAMSet.
I will add it.
There was a problem hiding this comment.
is backup always null if ms.error is true?
I think no, the visitor will not reassign the value of
backup. It only depends on the input.
I don't understand what you mean.
The only place where error is set to true is when if (backupAMSet == null).
I think the only way that can happen is if backup is null.
So in
if (ms.error) {
return backup;
}
I would expect that the returned value is always null.
Wouldn't the code be clearer if null is returned explicitly, possibly with a comment that backup == null?
If that argument is flawed, what am I missing?
There was a problem hiding this comment.
Thanks for clarification. See new commit d2baafd
framework/src/main/java/org/checkerframework/framework/flow/CFAbstractValue.java
Outdated
Show resolved
Hide resolved
…AbstractValue.java
wmdietl
changed the title
Fixed #662