Convert contrib/seg's bool-returning SQL functions to V1 call convention.

It appears that we can no longer get away with using V0 call convention
for bool-returning functions in newer versions of MSVC.  The compiler
seems to generate code that doesn't clear the higher-order bits of the
result register, causing the bool result Datum to often read as "true"
when "false" was intended.  This is not very surprising, since the
function thinks it's returning a bool-width result but fmgr_oldstyle
assumes that V0 functions return "char *"; what's surprising is that
that hack worked for so long on so many platforms.

The only functions of this description in core+contrib are in contrib/seg,
which we'd intentionally left mostly in V0 style to serve as a warning
canary if V0 call convention breaks.  We could imagine hacking things
so that they're still V0 (we'd have to redeclare the bool-returning
functions as returning some suitably wide integer type, like size_t,
at the C level).  But on the whole it seems better to convert 'em to V1.
We can still leave the pointer- and int-returning functions in V0 style,
so that the test coverage isn't gone entirely.

Back-patch to 9.5, since our intention is to support VS2015 in 9.5
and later.  There's no SQL-level change in the functions' behavior
so back-patching should be safe enough.

Discussion: <22094.1461273324@sss.pgh.pa.us>

Michael Paquier, adjusted some by me

diff --git a/contrib/seg/seg.c b/contrib/seg/seg.c
index c6c082b8ea6d0d04098bc7224f4229c66e5e2746..af1c592bb959387979a68df2e5ba8183fa17116f 100644 (file)
--- a/contrib/seg/seg.c
+++ b/contrib/seg/seg.c
@@ -34,6 +34,18 @@ typedef struct
        SEG                *data;
 } gseg_picksplit_item;
 
+/*
+ * Declarations for SQL-visible functions.
+ *
+ * Note: many of these functions have intentionally been left using V0 call
+ * convention, as a means of testing that that still works.  However, we had
+ * to convert functions taking or returning float4 to V1 convention, as it was
+ * otherwise too painful to deal with both pass-by-val and pass-by-ref cases.
+ * Also, on some modern platforms V0 functions returning bool do not work
+ * (because the compiler doesn't ensure that high-order bits of the
+ * pointer-sized result are zeroed), so those have been converted to V1 also.
+ */
+
 /*
 ** Input/Output routines
 */
@@ -47,52 +59,54 @@ PG_FUNCTION_INFO_V1(seg_center);
 /*
 ** GiST support methods
 */
-bool gseg_consistent(GISTENTRY *entry,
-                               SEG *query,
-                               StrategyNumber strategy,
-                               Oid subtype,
-                               bool *recheck);
+PG_FUNCTION_INFO_V1(gseg_consistent);
 GISTENTRY  *gseg_compress(GISTENTRY *entry);
 GISTENTRY  *gseg_decompress(GISTENTRY *entry);
 float     *gseg_penalty(GISTENTRY *origentry, GISTENTRY *newentry, float *result);
 GIST_SPLITVEC *gseg_picksplit(GistEntryVector *entryvec, GIST_SPLITVEC *v);
-bool           gseg_leaf_consistent(SEG *key, SEG *query, StrategyNumber strategy);
-bool           gseg_internal_consistent(SEG *key, SEG *query, StrategyNumber strategy);
+static bool gseg_leaf_consistent(SEG *key, SEG *query, StrategyNumber strategy);
+static bool gseg_internal_consistent(SEG *key, SEG *query, StrategyNumber strategy);
 SEG               *gseg_union(GistEntryVector *entryvec, int *sizep);
-SEG               *gseg_binary_union(SEG *r1, SEG *r2, int *sizep);
+static SEG *gseg_binary_union(SEG *r1, SEG *r2, int *sizep);
 bool      *gseg_same(SEG *b1, SEG *b2, bool *result);
 
 
 /*
 ** R-tree support functions
 */
-bool           seg_same(SEG *a, SEG *b);
-bool           seg_contains_int(SEG *a, int *b);
-bool           seg_contains_float4(SEG *a, float4 *b);
-bool           seg_contains_float8(SEG *a, float8 *b);
-bool           seg_contains(SEG *a, SEG *b);
-bool           seg_contained(SEG *a, SEG *b);
-bool           seg_overlap(SEG *a, SEG *b);
-bool           seg_left(SEG *a, SEG *b);
-bool           seg_over_left(SEG *a, SEG *b);
-bool           seg_right(SEG *a, SEG *b);
-bool           seg_over_right(SEG *a, SEG *b);
+PG_FUNCTION_INFO_V1(seg_same);
+PG_FUNCTION_INFO_V1(seg_contains);
+PG_FUNCTION_INFO_V1(seg_contained);
+PG_FUNCTION_INFO_V1(seg_overlap);
+PG_FUNCTION_INFO_V1(seg_left);
+PG_FUNCTION_INFO_V1(seg_over_left);
+PG_FUNCTION_INFO_V1(seg_right);
+PG_FUNCTION_INFO_V1(seg_over_right);
+static bool seg_same_internal(SEG *a, SEG *b);
+static bool seg_contains_internal(SEG *a, SEG *b);
+static bool seg_contained_internal(SEG *a, SEG *b);
+static bool seg_overlap_internal(SEG *a, SEG *b);
+static bool seg_left_internal(SEG *a, SEG *b);
+static bool seg_over_left_internal(SEG *a, SEG *b);
+static bool seg_right_internal(SEG *a, SEG *b);
+static bool seg_over_right_internal(SEG *a, SEG *b);
 SEG               *seg_union(SEG *a, SEG *b);
 SEG               *seg_inter(SEG *a, SEG *b);
-void           rt_seg_size(SEG *a, float *sz);
+static void rt_seg_size(SEG *a, float *sz);
 
 /*
 ** Various operators
 */
 int32          seg_cmp(SEG *a, SEG *b);
-bool           seg_lt(SEG *a, SEG *b);
-bool           seg_le(SEG *a, SEG *b);
-bool           seg_gt(SEG *a, SEG *b);
-bool           seg_ge(SEG *a, SEG *b);
-bool           seg_different(SEG *a, SEG *b);
+
+PG_FUNCTION_INFO_V1(seg_lt);
+PG_FUNCTION_INFO_V1(seg_le);
+PG_FUNCTION_INFO_V1(seg_gt);
+PG_FUNCTION_INFO_V1(seg_ge);
+PG_FUNCTION_INFO_V1(seg_different);
 
 /*
-** Auxiliary funxtions
+** Auxiliary functions
 */
 static int     restore(char *s, float val, int n);
 
@@ -193,13 +207,17 @@ seg_upper(PG_FUNCTION_ARGS)
 ** the predicate x op query == FALSE, where op is the oper
 ** corresponding to strategy in the pg_amop table.
 */
-bool
-gseg_consistent(GISTENTRY *entry,
-                               SEG *query,
-                               StrategyNumber strategy,
-                               Oid subtype,
-                               bool *recheck)
+Datum
+gseg_consistent(PG_FUNCTION_ARGS)
 {
+       GISTENTRY  *entry = (GISTENTRY *) PG_GETARG_POINTER(0);
+       SEG                *query = (SEG *) PG_GETARG_POINTER(1);
+       StrategyNumber strategy = (StrategyNumber) PG_GETARG_UINT16(2);
+
+       /* Oid          subtype = PG_GETARG_OID(3); */
+       bool       *recheck = (bool *) PG_GETARG_POINTER(4);
+       bool            result;
+
        /* All cases served by this function are exact */
        *recheck = false;
 
@@ -208,9 +226,13 @@ gseg_consistent(GISTENTRY *entry,
         * gseg_leaf_consistent
         */
        if (GIST_LEAF(entry))
-               return (gseg_leaf_consistent((SEG *) DatumGetPointer(entry->key), query, strategy));
+               result = gseg_leaf_consistent((SEG *) DatumGetPointer(entry->key),
+                                                                         query, strategy);
        else
-               return (gseg_internal_consistent((SEG *) DatumGetPointer(entry->key), query, strategy));
+               result = gseg_internal_consistent((SEG *) DatumGetPointer(entry->key),
+                                                                                 query, strategy);
+
+       PG_RETURN_BOOL(result);
 }
 
 /*
@@ -396,7 +418,7 @@ gseg_picksplit(GistEntryVector *entryvec,
 bool *
 gseg_same(SEG *b1, SEG *b2, bool *result)
 {
-       if (seg_same(b1, b2))
+       if (seg_same_internal(b1, b2))
                *result = TRUE;
        else
                *result = FALSE;
@@ -411,7 +433,7 @@ gseg_same(SEG *b1, SEG *b2, bool *result)
 /*
 ** SUPPORT ROUTINES
 */
-bool
+static bool
 gseg_leaf_consistent(SEG *key,
                                         SEG *query,
                                         StrategyNumber strategy)
@@ -425,30 +447,30 @@ gseg_leaf_consistent(SEG *key,
        switch (strategy)
        {
                case RTLeftStrategyNumber:
-                       retval = (bool) seg_left(key, query);
+                       retval = seg_left_internal(key, query);
                        break;
                case RTOverLeftStrategyNumber:
-                       retval = (bool) seg_over_left(key, query);
+                       retval = seg_over_left_internal(key, query);
                        break;
                case RTOverlapStrategyNumber:
-                       retval = (bool) seg_overlap(key, query);
+                       retval = seg_overlap_internal(key, query);
                        break;
                case RTOverRightStrategyNumber:
-                       retval = (bool) seg_over_right(key, query);
+                       retval = seg_over_right_internal(key, query);
                        break;
                case RTRightStrategyNumber:
-                       retval = (bool) seg_right(key, query);
+                       retval = seg_right_internal(key, query);
                        break;
                case RTSameStrategyNumber:
-                       retval = (bool) seg_same(key, query);
+                       retval = seg_same_internal(key, query);
                        break;
                case RTContainsStrategyNumber:
                case RTOldContainsStrategyNumber:
-                       retval = (bool) seg_contains(key, query);
+                       retval = seg_contains_internal(key, query);
                        break;
                case RTContainedByStrategyNumber:
                case RTOldContainedByStrategyNumber:
-                       retval = (bool) seg_contained(key, query);
+                       retval = seg_contained_internal(key, query);
                        break;
                default:
                        retval = FALSE;
@@ -456,7 +478,7 @@ gseg_leaf_consistent(SEG *key,
        return (retval);
 }
 
-bool
+static bool
 gseg_internal_consistent(SEG *key,
                                                 SEG *query,
                                                 StrategyNumber strategy)
@@ -470,28 +492,28 @@ gseg_internal_consistent(SEG *key,
        switch (strategy)
        {
                case RTLeftStrategyNumber:
-                       retval = (bool) !seg_over_right(key, query);
+                       retval = !seg_over_right_internal(key, query);
                        break;
                case RTOverLeftStrategyNumber:
-                       retval = (bool) !seg_right(key, query);
+                       retval = !seg_right_internal(key, query);
                        break;
                case RTOverlapStrategyNumber:
-                       retval = (bool) seg_overlap(key, query);
+                       retval = seg_overlap_internal(key, query);
                        break;
                case RTOverRightStrategyNumber:
-                       retval = (bool) !seg_left(key, query);
+                       retval = !seg_left_internal(key, query);
                        break;
                case RTRightStrategyNumber:
-                       retval = (bool) !seg_over_left(key, query);
+                       retval = !seg_over_left_internal(key, query);
                        break;
                case RTSameStrategyNumber:
                case RTContainsStrategyNumber:
                case RTOldContainsStrategyNumber:
-                       retval = (bool) seg_contains(key, query);
+                       retval = seg_contains_internal(key, query);
                        break;
                case RTContainedByStrategyNumber:
                case RTOldContainedByStrategyNumber:
-                       retval = (bool) seg_overlap(key, query);
+                       retval = seg_overlap_internal(key, query);
                        break;
                default:
                        retval = FALSE;
@@ -499,7 +521,7 @@ gseg_internal_consistent(SEG *key,
        return (retval);
 }
 
-SEG *
+static SEG *
 gseg_binary_union(SEG *r1, SEG *r2, int *sizep)
 {
        SEG                *retval;
@@ -511,32 +533,68 @@ gseg_binary_union(SEG *r1, SEG *r2, int *sizep)
 }
 
 
-bool
-seg_contains(SEG *a, SEG *b)
+Datum
+seg_contains(PG_FUNCTION_ARGS)
+{
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_contains_internal(a, b));
+}
+
+static bool
+seg_contains_internal(SEG *a, SEG *b)
 {
        return ((a->lower <= b->lower) && (a->upper >= b->upper));
 }
 
-bool
-seg_contained(SEG *a, SEG *b)
+Datum
+seg_contained(PG_FUNCTION_ARGS)
 {
-       return (seg_contains(b, a));
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_contained_internal(a, b));
+}
+
+static bool
+seg_contained_internal(SEG *a, SEG *b)
+{
+       return (seg_contains_internal(b, a));
 }
 
 /*****************************************************************************
  * Operator class for R-tree indexing
  *****************************************************************************/
 
-bool
-seg_same(SEG *a, SEG *b)
+Datum
+seg_same(PG_FUNCTION_ARGS)
+{
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_same_internal(a, b));
+}
+
+static bool
+seg_same_internal(SEG *a, SEG *b)
 {
        return seg_cmp(a, b) == 0;
 }
 
 /*     seg_overlap -- does a overlap b?
  */
-bool
-seg_overlap(SEG *a, SEG *b)
+Datum
+seg_overlap(PG_FUNCTION_ARGS)
+{
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_overlap_internal(a, b));
+}
+
+static bool
+seg_overlap_internal(SEG *a, SEG *b)
 {
        return (
                        ((a->upper >= b->upper) && (a->lower <= b->upper))
@@ -547,32 +605,68 @@ seg_overlap(SEG *a, SEG *b)
 
 /*     seg_overleft -- is the right edge of (a) located at or left of the right edge of (b)?
  */
-bool
-seg_over_left(SEG *a, SEG *b)
+Datum
+seg_over_left(PG_FUNCTION_ARGS)
+{
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_over_left_internal(a, b));
+}
+
+static bool
+seg_over_left_internal(SEG *a, SEG *b)
 {
        return (a->upper <= b->upper);
 }
 
 /*     seg_left -- is (a) entirely on the left of (b)?
  */
-bool
-seg_left(SEG *a, SEG *b)
+Datum
+seg_left(PG_FUNCTION_ARGS)
+{
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_left_internal(a, b));
+}
+
+static bool
+seg_left_internal(SEG *a, SEG *b)
 {
        return (a->upper < b->lower);
 }
 
 /*     seg_right -- is (a) entirely on the right of (b)?
  */
-bool
-seg_right(SEG *a, SEG *b)
+Datum
+seg_right(PG_FUNCTION_ARGS)
+{
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_right_internal(a, b));
+}
+
+static bool
+seg_right_internal(SEG *a, SEG *b)
 {
        return (a->lower > b->upper);
 }
 
 /*     seg_overright -- is the left edge of (a) located at or right of the left edge of (b)?
  */
-bool
-seg_over_right(SEG *a, SEG *b)
+Datum
+seg_over_right(PG_FUNCTION_ARGS)
+{
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_over_right_internal(a, b));
+}
+
+static bool
+seg_over_right_internal(SEG *a, SEG *b)
 {
        return (a->lower >= b->lower);
 }
@@ -655,7 +749,7 @@ seg_inter(SEG *a, SEG *b)
        return (n);
 }
 
-void
+static void
 rt_seg_size(SEG *a, float *size)
 {
        if (a == (SEG *) NULL || a->upper <= a->lower)
@@ -797,36 +891,50 @@ seg_cmp(SEG *a, SEG *b)
        return 0;
 }
 
-bool
-seg_lt(SEG *a, SEG *b)
+Datum
+seg_lt(PG_FUNCTION_ARGS)
 {
-       return seg_cmp(a, b) < 0;
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_cmp(a, b) < 0);
 }
 
-bool
-seg_le(SEG *a, SEG *b)
+Datum
+seg_le(PG_FUNCTION_ARGS)
 {
-       return seg_cmp(a, b) <= 0;
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_cmp(a, b) <= 0);
 }
 
-bool
-seg_gt(SEG *a, SEG *b)
+Datum
+seg_gt(PG_FUNCTION_ARGS)
 {
-       return seg_cmp(a, b) > 0;
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_cmp(a, b) > 0);
 }
 
-bool
-seg_ge(SEG *a, SEG *b)
+Datum
+seg_ge(PG_FUNCTION_ARGS)
 {
-       return seg_cmp(a, b) >= 0;
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
+
+       PG_RETURN_BOOL(seg_cmp(a, b) >= 0);
 }
 
-bool
-seg_different(SEG *a, SEG *b)
+Datum
+seg_different(PG_FUNCTION_ARGS)
 {
-       return seg_cmp(a, b) != 0;
-}
+       SEG                *a = (SEG *) PG_GETARG_POINTER(0);
+       SEG                *b = (SEG *) PG_GETARG_POINTER(1);
 
+       PG_RETURN_BOOL(seg_cmp(a, b) != 0);
+}
 
 
 /*****************************************************************************
@@ -985,24 +1093,6 @@ restore(char *result, float val, int n)
 ** Miscellany
 */
 
-bool
-seg_contains_int(SEG *a, int *b)
-{
-       return ((a->lower <= *b) && (a->upper >= *b));
-}
-
-bool
-seg_contains_float4(SEG *a, float4 *b)
-{
-       return ((a->lower <= *b) && (a->upper >= *b));
-}
-
-bool
-seg_contains_float8(SEG *a, float8 *b)
-{
-       return ((a->lower <= *b) && (a->upper >= *b));
-}
-
 /* find out the number of significant digits in a string representing
  * a floating point number
  */