gnumeric r17079 - in trunk: . plugins/fn-logical src

[mortenw svn gnome org]

Author: mortenw
Date: Tue Jan 20 16:18:54 2009
New Revision: 17079
URL: http://svn.gnome.org/viewvc/gnumeric?rev=17079&view=rev

Log:
2009-01-20  Morten Welinder  <terra gnome org>

	* src/func.c (function_call_with_exprs): If we are not doing
	implicit iteration, use gnumeric_if instead of gnumeric_if2.
	Fixes #326595.

	* src/func-builtin.c (gnumeric_if): Import from fn-logical.
	Change help to new style.
	(gnumeric_if2): Variant that takes the argument un-eval'd.



Modified:
   trunk/ChangeLog
   trunk/NEWS
   trunk/plugins/fn-logical/functions.c
   trunk/plugins/fn-logical/plugin.xml.in
   trunk/src/func-builtin.c
   trunk/src/func-builtin.h
   trunk/src/func.c

Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS	(original)
+++ trunk/NEWS	Tue Jan 20 16:18:54 2009
@@ -72,6 +72,7 @@
 	* Fix inconsistentcy in searching with respect to formats.
 	* Fix issue with BASE.   [#567252]
 	* Fix XLS import criticals.  [#567823]
+	* Don't evaluate both branches of IF.  [#326595]
 
 --------------------------------------------------------------------------
 Gnumeric 1.9.3

Modified: trunk/plugins/fn-logical/functions.c
==============================================================================
--- trunk/plugins/fn-logical/functions.c	(original)
+++ trunk/plugins/fn-logical/functions.c	Tue Jan 20 16:18:54 2009
@@ -264,47 +264,6 @@
 
 /***************************************************************************/
 
-static GnmFuncHelp const help_if[] = {
-	{ GNM_FUNC_HELP_OLD,
-	F_("@FUNCTION=IF\n"
-	   "@SYNTAX=IF(condition[,if-true,if-false])\n"
-
-	   "@DESCRIPTION="
-	   "IF function can be used to evaluate conditionally other "
-	   "expressions. IF evaluates @condition.  If @condition returns a "
-	   "non-zero value the result of the IF expression is the @if-true "
-	   "expression, otherwise IF evaluates to the value of @if-false.\n"
-	   "\n"
-	   "* If omitted @if-true defaults to TRUE and @if-false to FALSE.\n"
-	   "* This function is Excel compatible.\n"
-	   "\n"
-	   "@EXAMPLES=\n"
-	   "IF(FALSE,TRUE,FALSE) equals FALSE.\n"
-	   "\n"
-	   "@SEEALSO=")
-	},
-	{ GNM_FUNC_HELP_END }
-};
-
-static GnmValue *
-gnumeric_if (GnmFuncEvalInfo *ei, GnmValue const * const *args)
-{
-	gboolean err;
-	int res = value_get_as_bool (args[0], &err) ? 1 : 2;
-
-	if (args[res])
-		return value_dup (args[res]);
-
-	if (ei->func_call->argc < res + 1)
-		/* arg-not-there: default to TRUE/FALSE.  */
-		return value_new_bool (res == 1);
-	else
-		/* arg blank: default to 0.  */
-		return value_new_int (0);
-}
-
-/***************************************************************************/
-
 static GnmFuncHelp const help_iferror[] = {
 	{ GNM_FUNC_HELP_NAME, F_("IFERROR:Test for error.") },
 	{ GNM_FUNC_HELP_ARG, F_("x:value to test for error.") },
@@ -385,10 +344,6 @@
 	  NULL, NULL, NULL, NULL,
 	  GNM_FUNC_SIMPLE + GNM_FUNC_AUTO_UNITLESS,
 	  GNM_FUNC_IMPL_STATUS_COMPLETE, GNM_FUNC_TEST_STATUS_BASIC },
-	{ "if", "b|EE", N_("condition,if true,if false"), help_if,
-	  gnumeric_if, NULL, NULL, NULL, NULL,
-	  GNM_FUNC_SIMPLE + GNM_FUNC_AUTO_SECOND,
-	  GNM_FUNC_IMPL_STATUS_COMPLETE, GNM_FUNC_TEST_STATUS_BASIC },
 	{ "iferror", "EE", N_("value,value"), help_iferror,
 	  gnumeric_iferror, NULL, NULL, NULL, NULL,
 	  GNM_FUNC_SIMPLE, GNM_FUNC_IMPL_STATUS_COMPLETE, GNM_FUNC_TEST_STATUS_NO_TESTSUITE },

Modified: trunk/plugins/fn-logical/plugin.xml.in
==============================================================================
--- trunk/plugins/fn-logical/plugin.xml.in	(original)
+++ trunk/plugins/fn-logical/plugin.xml.in	Tue Jan 20 16:18:54 2009
@@ -15,7 +15,6 @@
 				<function name="or"/>
 				<function name="xor"/>
 				<function name="not"/>
-				<function name="if"/>
 				<function name="iferror"/>
 				<function name="true"/>
 				<function name="false"/>

Modified: trunk/src/func-builtin.c
==============================================================================
--- trunk/src/func-builtin.c	(original)
+++ trunk/src/func-builtin.c	Tue Jan 20 16:18:54 2009
@@ -272,10 +272,86 @@
 
 	return res;
 }
+
+/***************************************************************************/
+
+static GnmFuncHelp const help_if[] = {
+	{ GNM_FUNC_HELP_NAME, N_("IF:conditional expression.") },
+	{ GNM_FUNC_HELP_ARG, N_("cond:condition.") },
+	{ GNM_FUNC_HELP_ARG, N_("trueval:value to use if condition is true.") },
+	{ GNM_FUNC_HELP_ARG, N_("falseval:value to use if condition is false.") },
+	{ GNM_FUNC_HELP_DESCRIPTION, N_("This function first evaluates the condition.  If the result is true, it will then evaluate and return the second argument.  Otherwise, it will evaluate and return the last argument.") },
+	{ GNM_FUNC_HELP_SEEALSO, "AND,OR,XOR,NOT,IFERROR" },
+	{ GNM_FUNC_HELP_END }
+};
+
+GnmValue *
+gnumeric_if (GnmFuncEvalInfo *ei, GnmValue const * const *args)
+{
+	gboolean err;
+	int res = value_get_as_bool (args[0], &err) ? 1 : 2;
+
+	if (args[res])
+		return value_dup (args[res]);
+
+	if (ei->func_call->argc < res + 1)
+		/* arg-not-there: default to TRUE/FALSE.  */
+		return value_new_bool (res == 1);
+	else
+		/* arg blank: default to 0.  */
+		return value_new_int (0);
+}
+
+
+GnmValue *
+gnumeric_if2 (GnmFuncEvalInfo *ei, int argc, GnmExprConstPtr const *argv)
+{
+	gboolean err;
+	int i, branch;
+	GnmValue *args[3];
+	GnmValue *res;
+
+	g_return_val_if_fail (argc >= 1 && argc <= 3,
+			      value_new_error_VALUE (ei->pos));
+
+	/*
+	 * In this version of IF, we evaluate the arguments ourselves,
+	 * the call the regular IF.  However, arguments we do not need
+	 * we do not evaluate.
+	 *
+	 * IF is sometimes used to avoid expensive calculations.  Always
+	 * computing both branches destroys that intent.  See bug 326595.
+	 */
+
+	/* Evaluate condition.  */
+	res = gnm_expr_eval (argv[0], ei->pos, 0);
+	if (VALUE_IS_ERROR (res))
+		return res;
+	args[0] = res;
+
+	branch = value_get_as_bool (args[0], &err) ? 1 : 2;
+	for (i = 1; i <= 2; i++) {
+		args[i] = argc > i
+			? (branch == i ?
+			   gnm_expr_eval (argv[branch], ei->pos, 0)
+			   : value_new_empty ())
+			: NULL;
+	}
+
+	res = gnumeric_if (ei, (GnmValue const * const *)args);
+
+	for (i = 0; i <= 2; i++)
+		if (args[i])
+			value_release (args[i]);
+
+	return res;
+}
+
 /***************************************************************************/
 
 static GnmFuncGroup *math_group = NULL;
 static GnmFuncGroup *gnumeric_group = NULL;
+static GnmFuncGroup *logic_group = NULL;
 
 void
 func_builtin_init (void)
@@ -306,6 +382,12 @@
 			GNM_FUNC_IMPL_STATUS_UNIQUE_TO_GNUMERIC,
 			GNM_FUNC_TEST_STATUS_EXHAUSTIVE
 		},
+		{	"if", "b|EE", N_("condition,if true,if false"),
+			help_if, gnumeric_if, NULL,
+			NULL, NULL, NULL,
+			GNM_FUNC_SIMPLE + GNM_FUNC_AUTO_SECOND,
+			GNM_FUNC_IMPL_STATUS_COMPLETE,
+			GNM_FUNC_TEST_STATUS_BASIC },
 		{ NULL }
 	};
 
@@ -316,6 +398,9 @@
 	gnumeric_group = gnm_func_group_fetch (N_("Gnumeric"));
 	gnm_func_add (gnumeric_group, builtins + 2);
 	gnm_func_add (gnumeric_group, builtins + 3);
+
+	logic_group = gnm_func_group_fetch (N_("Logic"));
+	gnm_func_add (logic_group, builtins + 4);
 }
 
 static void
@@ -332,4 +417,5 @@
 {
 	shutdown_cat (math_group);
 	shutdown_cat (gnumeric_group);
+	shutdown_cat (logic_group);
 }

Modified: trunk/src/func-builtin.h
==============================================================================
--- trunk/src/func-builtin.h	(original)
+++ trunk/src/func-builtin.h	Tue Jan 20 16:18:54 2009
@@ -8,6 +8,8 @@
 
 GnmValue *gnumeric_sum     (GnmFuncEvalInfo *ei, int argc, GnmExprConstPtr const *argv);
 GnmValue *gnumeric_product (GnmFuncEvalInfo *ei, int argc, GnmExprConstPtr const *argv);
+GnmValue *gnumeric_if      (GnmFuncEvalInfo *ei, GnmValue const * const *args);
+GnmValue *gnumeric_if2     (GnmFuncEvalInfo *ei, int argc, GnmExprConstPtr const *argv);
 
 void func_builtin_init (void);
 void func_builtin_shutdown (void);

Modified: trunk/src/func.c
==============================================================================
--- trunk/src/func.c	(original)
+++ trunk/src/func.c	Tue Jan 20 16:18:54 2009
@@ -1010,6 +1010,8 @@
 			value_release (values [i]);
 }
 
+/* ------------------------------------------------------------------------- */
+
 /**
  * function_call_with_exprs:
  * @ei: EvalInfo containing valid fn_def!
@@ -1053,6 +1055,11 @@
 		      (flags & GNM_EXPR_EVAL_PERMIT_NON_SCALAR))
 		? 0 : -1;
 
+	/* Optimization for IF when implicit iteration is not used.  */
+	if (ei->func_call->func->fn.args.func == gnumeric_if &&
+	    iter_count == -1)
+		return gnumeric_if2 (ei, argc, argv);
+
 	for (i = 0; i < argc; i++) {
 		char arg_type = fn_def->fn.args.arg_types[i];
 		/* expr is always non-null, missing args are encoded as