Annotated OpenIntToFieldHashMap.java and FastMathCalc.java #3
Conversation
There was a problem hiding this comment.
Minor nitpicks, mostly. When you fix the issues I mentioned, this should go to @panacekcz.
| private static void buildSinCosTables(double[] SINE_TABLE_A, double[] SINE_TABLE_B, | ||
| double[] COSINE_TABLE_A, double[] COSINE_TABLE_B, | ||
| int SINE_TABLE_LEN, double[] TANGENT_TABLE_A, double[] TANGENT_TABLE_B) { | ||
| @SuppressWarnings({"unused", "index:array.access.unsafe.high"}) // #1: i is odd as checked by the previous if-else, so i/2 + 1 < SINE_TABLE_LEN |
There was a problem hiding this comment.
The fact you've given about i doesn't seem to follow from i being odd. I think the fact that i < SINE_TABLE_LEN (the loop condition) is also necessary, so you should mention it in the comment here.
| @@ -83,11 +83,11 @@ public PivotingStrategyInterface getPivotingStrategy() { | |||
| @SuppressWarnings({"index:array.access.unsafe.low", "index:argument.type.incompatible"}) /* | |||
| #1: node is always @NonNegative as it is changed only in #0.1 where it is minimum of 2*node + 1(or 2) and either pivotsHeap.length or end | |||
| pivotsHeap.length is @NonNegative as it is a length, end is also @NonNegative as it is initialized 0 and changed only in #0.2 where end = pivot >= k that is @NonNegative | |||
| #3: begin and end are @NonNegative as t is assign | |||
| #3: begin and end are @NonNegative as it is initialised with @NonNegative values in #0. and changed to only @NonNegative values in #0.2 and #0.4 | |||
There was a problem hiding this comment.
I think this comment is supposed to say "#0.3" instead of "#0."?
There was a problem hiding this comment.
I also think you could eliminate some (maybe all?) of these errors by annotating the data that flows into pivot as @NonNegative: pivotsHeap should be final @NonNegative int[], and partition should return @NonNegative
| @@ -415,6 +430,10 @@ private T doRemove(int index) { | |||
| * @param value value to put in the map | |||
| * @return previous value associated with the key | |||
| */ | |||
| @SuppressWarnings({"index:array.access.unsafe.low", "index:array.access.unsafe.high"}) /* | |||
| #1: findInsertionIndex() returns (<variable> & mask) or changeIndexSign(<variable> & mask) whose magnitude is surely @LessThan({"this.values.length", "this.states.length", "this.keys.length"}) | |||
There was a problem hiding this comment.
I suggest avoiding intensifiers such as "surely", "obviously", "clearly", and "very" in technical writing (such as suppress warning justifications). Adverbs in general are often a mistake in technical writing, because you want to be short and specific.
|
@kelloggm @panacekcz made the suggested changes |
|
@panacekcz I think this is ready for your review. |
| @@ -134,7 +137,7 @@ public double select(final double[] work, final int[] pivotsHeap, final @IndexFo | |||
| * @param pivot initial index of the pivot | |||
| * @return index of the pivot after partition | |||
| */ | |||
| private @NonNegative int partition(final double[] work, final @IndexFor("#1") int begin, final @IndexOrHigh("#1") int end, final @IndexFor("#1") int pivot) { | |||
| private @NonNegative @IndexFor("#1") int partition(final double[] work, final @IndexFor("#1") int begin, final @IndexOrHigh("#1") int end, final @IndexFor("#1") int pivot) { | |||
There was a problem hiding this comment.
@NonNegative should be implied by @IndexFor
|
@panacekcz Made the change you suggested |
No description provided.