Refactor gmp_fact()

Author: Girgias

ext/gmp/gmp.c

@@ -1197,35 +1197,24 @@ ZEND_FUNCTION(gmp_abs)
 ZEND_FUNCTION(gmp_fact)
 {
 	zval *a_arg;
+	mpz_ptr gmpnum;
+	mpz_ptr gmpnum_result;
+	gmp_temp_t temp_a;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &a_arg) == FAILURE){
 		RETURN_THROWS();
 	}
 
-	if (IS_GMP(a_arg)) {
-		mpz_ptr gmpnum_tmp = GET_GMP_FROM_ZVAL(a_arg);
-		if (mpz_sgn(gmpnum_tmp) < 0) {
-			zend_argument_value_error(1, "must be greater than or equal to 0");
-			RETURN_THROWS();
-		}
-	} else {
-		/* Use convert_to_number first to detect getting non-integer */
-		convert_scalar_to_number(a_arg);
-		if (Z_TYPE_P(a_arg) != IS_LONG) {
-			convert_to_long(a_arg);
-			if (Z_LVAL_P(a_arg) >= 0) {
-				/* Only warn if we'll make it past the non-negative check */
-				// TODO: promote? Also I don't get this
-				php_error_docref(NULL, E_WARNING, "Number has to be an integer");
-			}
-		}
-		if (Z_LVAL_P(a_arg) < 0) {
-			zend_argument_value_error(1, "must be greater than or equal to 0");
-			RETURN_THROWS();
-		}
+	FETCH_GMP_ZVAL(gmpnum, a_arg, temp_a, 1);
+	FREE_GMP_TEMP(temp_a);
+
+	if (mpz_sgn(gmpnum) < 0) {
+		zend_argument_value_error(1, "must be greater than or equal to 0");
+		RETURN_THROWS();
 	}
 
-	gmp_zval_unary_ui_op(return_value, a_arg, mpz_fac_ui);
+	INIT_GMP_RETVAL(gmpnum_result);
+	mpz_fac_ui(gmpnum_result, zval_get_long(a_arg));
 }
 /* }}} */
 

ext/gmp/tests/gmp_fact.phpt

@@ -6,7 +6,11 @@ gmp_fact() basic tests
 <?php
 
 var_dump(gmp_strval(gmp_fact(0)));
-var_dump(gmp_strval(gmp_fact("")));
+try {
+    var_dump(gmp_strval(gmp_fact("")));
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 var_dump(gmp_strval(gmp_fact("0")));
 try {
     var_dump(gmp_strval(gmp_fact("-1")));
@@ -18,7 +22,13 @@ try {
 } catch (\ValueError $e) {
     echo $e->getMessage() . \PHP_EOL;
 }
-var_dump(gmp_strval(gmp_fact(1.1)));
+
+try {
+    var_dump(gmp_strval(gmp_fact(1.1)));
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
+
 var_dump(gmp_strval(gmp_fact(20)));
 var_dump(gmp_strval(gmp_fact("50")));
 var_dump(gmp_strval(gmp_fact("10")));
@@ -33,33 +43,26 @@ try {
     echo $e->getMessage() . \PHP_EOL;
 }
 
-var_dump(gmp_fact(array()));
-var_dump(gmp_strval(gmp_fact(array())));
+try {
+    var_dump(gmp_fact(array()));
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 
 echo "Done\n";
 ?>
---EXPECTF--
-string(1) "1"
+--EXPECT--
 string(1) "1"
+gmp_fact(): Argument #1 ($a) must be an integer string in base 10
 string(1) "1"
 gmp_fact(): Argument #1 ($a) must be greater than or equal to 0
 gmp_fact(): Argument #1 ($a) must be greater than or equal to 0
-
-Warning: gmp_fact(): Number has to be an integer in %s on line %d
-string(1) "1"
+gmp_fact(): Argument #1 ($a) must be of type bool|int|string|GMP, float given
 string(19) "2432902008176640000"
 string(65) "30414093201713378043612608166064768844377641568960512000000000000"
 string(7) "3628800"
 string(1) "1"
 string(9) "479001600"
 gmp_fact(): Argument #1 ($a) must be greater than or equal to 0
-
-Warning: gmp_fact(): Number has to be an integer in %s on line %d
-object(GMP)#%d (1) {
-  ["num"]=>
-  string(1) "1"
-}
-
-Warning: gmp_fact(): Number has to be an integer in %s on line %d
-string(1) "1"
+gmp_fact(): Argument #1 ($a) must be of type bool|int|string|GMP, array given
 Done