Skip to content

Commit 887dab1

Browse files
committed
Tighten implementation of SnippetTiebreaker
The comparator could return a premature result if one Snippet or both being compared has a null implementor name, resulting in an output ordering that could depend on the sequence of comparisons performed. The case is unlikely to be seen in practice, as an implementor name can only be made null by explicit implementor="" in an annotation or -Addr.implementor=- when invoking the Java compiler. Nonetheless, the tiebreaker method can be made more simple and convincing by taking advantage of the Java 8 additions to Comparator. Addresses #527. For consistency's sake, also Comparator-ize TypeTiebreaker.
1 parent ddc9348 commit 887dab1

File tree

2 files changed

+35
-40
lines changed

2 files changed

+35
-40
lines changed

pljava-api/src/main/java/org/postgresql/pljava/annotation/processing/SnippetTiebreaker.java

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,44 +12,37 @@
1212
*/
1313
package org.postgresql.pljava.annotation.processing;
1414

15+
import java.util.Arrays;
1516
import java.util.Comparator;
17+
import static java.util.Comparator.comparing;
18+
import static java.util.Comparator.naturalOrder;
19+
import static java.util.Comparator.nullsFirst;
1620

17-
import org.postgresql.pljava.sqlgen.Lexicals.Identifier;
21+
import org.postgresql.pljava.sqlgen.Lexicals.Identifier.Simple;
1822

1923
/**
2024
* Resolve ties in {@code Snippet} ordering in an arbitrary but deterministic
2125
* way, for use when {@code ddr.reproducible} is set.
2226
*/
2327
class SnippetTiebreaker implements Comparator<Vertex<Snippet>>
2428
{
29+
private static final Comparator<Vertex<Snippet>> VCMP;
30+
31+
static
32+
{
33+
Comparator<Snippet> scmp =
34+
comparing(Snippet::implementorName,
35+
nullsFirst(comparing(Simple::pgFolded, naturalOrder()))
36+
)
37+
.thenComparing(Snippet::deployStrings, Arrays::compare)
38+
.thenComparing(Snippet::undeployStrings, Arrays::compare);
39+
40+
VCMP = comparing(v -> v.payload, scmp);
41+
}
42+
2543
@Override
26-
public int compare( Vertex<Snippet> o1, Vertex<Snippet> o2)
44+
public int compare(Vertex<Snippet> o1, Vertex<Snippet> o2)
2745
{
28-
Snippet s1 = o1.payload;
29-
Snippet s2 = o2.payload;
30-
int diff;
31-
Identifier.Simple s1imp = s1.implementorName();
32-
Identifier.Simple s2imp = s2.implementorName();
33-
if ( null != s1imp && null != s2imp )
34-
{
35-
diff = s1imp.pgFolded().compareTo( s2imp.pgFolded());
36-
if ( 0 != diff )
37-
return diff;
38-
}
39-
else
40-
return null == s1imp ? -1 : 1;
41-
String[] ds1 = s1.deployStrings();
42-
String[] ds2 = s2.deployStrings();
43-
diff = ds1.length - ds2.length;
44-
if ( 0 != diff )
45-
return diff;
46-
for ( int i = 0 ; i < ds1.length ; ++ i )
47-
{
48-
diff = ds1[i].compareTo( ds2[i]);
49-
if ( 0 != diff )
50-
return diff;
51-
}
52-
assert s1 == s2 : "Two distinct Snippets compare equal by tiebreaker";
53-
return 0;
46+
return VCMP.compare(o1, o2);
5447
}
5548
}

pljava-api/src/main/java/org/postgresql/pljava/annotation/processing/TypeTiebreaker.java

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package org.postgresql.pljava.annotation.processing;
1414

1515
import java.util.Comparator;
16+
import static java.util.Comparator.comparing;
1617
import java.util.Map;
1718

1819
import javax.lang.model.type.TypeMirror;
@@ -24,22 +25,23 @@
2425
class TypeTiebreaker
2526
implements Comparator<Vertex<Map.Entry<TypeMirror, DBType>>>
2627
{
28+
private static final Comparator<Vertex<Map.Entry<TypeMirror, DBType>>> VCMP;
29+
30+
static
31+
{
32+
Comparator<Map.Entry<TypeMirror, DBType>> ecmp =
33+
comparing(
34+
(Map.Entry<TypeMirror, DBType> e) -> e.getValue().toString())
35+
.thenComparing(e -> e.getKey().toString());
36+
37+
VCMP = comparing(v -> v.payload, ecmp);
38+
}
39+
2740
@Override
2841
public int compare(
2942
Vertex<Map.Entry<TypeMirror, DBType>> o1,
3043
Vertex<Map.Entry<TypeMirror, DBType>> o2)
3144
{
32-
Map.Entry<TypeMirror, DBType> m1 = o1.payload;
33-
Map.Entry<TypeMirror, DBType> m2 = o2.payload;
34-
int diff =
35-
m1.getValue().toString().compareTo( m2.getValue().toString());
36-
if ( 0 != diff )
37-
return diff;
38-
diff = m1.getKey().toString().compareTo( m2.getKey().toString());
39-
if ( 0 != diff )
40-
return diff;
41-
assert
42-
m1 == m2 : "Two distinct type mappings compare equal by tiebreaker";
43-
return 0;
45+
return VCMP.compare(o1, o2);
4446
}
4547
}

0 commit comments

Comments
 (0)