Fix optimization hazard in gram.y's makeOrderedSetArgs(), redux.

tglsfdc · tglsfdc · commit e3868c7d59d4 · 2020-10-07T18:42:23.000-04:00
It appears that commit cf63c64, which intended to prevent misoptimization of the result-building step in makeOrderedSetArgs, didn't go far enough: buildfarm member hornet's version of xlc is now optimizing back to the old, broken behavior in which list_length(directargs) is fetched only after list_concat() has changed that value. I'm not entirely convinced whether that's an undeniable compiler bug or whether it can be justified by a sufficiently aggressive interpretation of C sequence points. So let's just change the code to make it harder to misinterpret. Back-patch to all supported versions, just in case. Discussion: https://postgr.es/m/1830491.1601944935@sss.pgh.pa.us
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
@@ -15812,7 +15812,7 @@ makeOrderedSetArgs(List *directargs, List *orderedargs,
 				   core_yyscan_t yyscanner)
 {
 	FunctionParameter *lastd = (FunctionParameter *) llast(directargs);
-	int			ndirectargs;
+	Value	   *ndirectargs;
 
 	/* No restriction unless last direct arg is VARIADIC */
 	if (lastd->mode == FUNC_PARAM_VARIADIC)
@@ -15836,10 +15836,10 @@ makeOrderedSetArgs(List *directargs, List *orderedargs,
 	}
 
 	/* don't merge into the next line, as list_concat changes directargs */
-	ndirectargs = list_length(directargs);
+	ndirectargs = makeInteger(list_length(directargs));
 
 	return list_make2(list_concat(directargs, orderedargs),
-					  makeInteger(ndirectargs));
+					  ndirectargs);
 }
 
 /* insertSelectOptions()

Original file line number	Diff line number	Diff line change
`@@ -15812,7 +15812,7 @@ makeOrderedSetArgs(List directargs, List orderedargs,`
`15812`	`15812`	`core_yyscan_t yyscanner)`
`15813`	`15813`	`{`
`15814`	`15814`	`FunctionParameter lastd = (FunctionParameter ) llast(directargs);`
`15815`		`- int ndirectargs;`
	`15815`	`+ Value *ndirectargs;`
`15816`	`15816`
`15817`	`15817`	`/* No restriction unless last direct arg is VARIADIC */`
`15818`	`15818`	`if (lastd->mode == FUNC_PARAM_VARIADIC)`
`@@ -15836,10 +15836,10 @@ makeOrderedSetArgs(List directargs, List orderedargs,`
`15836`	`15836`	`}`
`15837`	`15837`
`15838`	`15838`	`/* don't merge into the next line, as list_concat changes directargs */`
`15839`		`- ndirectargs = list_length(directargs);`
	`15839`	`+ ndirectargs = makeInteger(list_length(directargs));`
`15840`	`15840`
`15841`	`15841`	`return list_make2(list_concat(directargs, orderedargs),`
`15842`		`- makeInteger(ndirectargs));`
	`15842`	`+ ndirectargs);`
`15843`	`15843`	`}`
`15844`	`15844`
`15845`	`15845`	`/* insertSelectOptions()`