Conversation
|
@smillst Could you please add the missing documentation? Also, a test is failing. |
| /** Information collected about local variables (including method parameters). */ | ||
| protected final Map<LocalVariable, V> localVariableValues; | ||
|
|
||
| /** Stores after boolean variable assignment. */ |
There was a problem hiding this comment.
I wound find this comment more useful if it said more about the semantics, rather than when the field is first set. The map is from boolean local variables, and the mapped-to store indicates facts that are true depending on whether the boolean local variable is true or false.
| protected final Map<LocalVariable, BooleanVarStore<V, S>> booleanVarStores; | ||
|
|
||
| /** | ||
| * An object that contains the then and else store after a boolean variable assignment. |
There was a problem hiding this comment.
Here too, I think that "after a boolean variable assignment" is not the most important thing to document.
It would be helpful, here, to discuss the goal and the design.
| BooleanVarStore(S thenStore, S elseStore) { | ||
| this.thenStore = thenStore.copy(); | ||
| this.elseStore = elseStore.copy(); | ||
| thenStore.booleanVarStores.clear(); |
There was a problem hiding this comment.
It would be helpful to document the reason that these substores are cleared.
| } | ||
|
|
||
| for (Map.Entry<LocalVariable, BooleanVarStore<V, S>> e : other.booleanVarStores.entrySet()) { | ||
| // If a local variable appears in just one store, then the other store implicitly contains |
There was a problem hiding this comment.
Here and above, can you remark on whether this is done for correctness, or performance, or as a heuristic?
mernst
left a comment
mernst
left a comment
There was a problem hiding this comment.
This pull request looks good overall -- thanks!
However, it needs more documentation, both of its goal and its implementation.
|
Linking this comment on the previous PR so it doesn't get lost (regarding overhead): |
3 participants
This implementation isn't working as expected. In particular, there are unexpected errors in
checker/tests/nullness/Issue406.java.Fixes #406.