# HG changeset patch
# User Rik <rik@octave.org>
# Date 1521653918 25200
# Node ID bff4a7c7bc393fcc4250d83730af662b39f4e199
# Parent  ba9d37893822d3ec79771a8195b55871e968b0d4
Fix bsxfun when operating with float complex values (bug #53179).

* bsxfun.h (is_valid_bsxfun): Rename variables dx, dy to xdv, ydv for clarity.
* bsxfun.h (is_valid_inplace_bsxfun): Rename variables dr, dx to rdv, xdv for
clarity.

* bsxfun.cc: Alphabetize #include lists.
* bsxfun.cc (Fbsxfun): Change mistaken copy&paste block in initialization
(i == 0) and use variables have_FloatComplexNDArray, result_FloatComplexNDArray
rather than have_ComplexNDArray, result_ComplexNDArray when input is a float
and complex.  Also use float_complex_array_value() extractor.
Delete code for '|| have_FloatComplexNDArray' and mirror the code for 'have_NDArray'
to apply to the case of 'have_FloatNDArray'.  Re-order code so if test for
'have_NDArray' is first since it is the most common case.  Add missing BSXLOOP
case for FloatComplexNDArray and single values.  Add BIST test for bug #53179.

diff -r ba9d37893822 -r bff4a7c7bc39 libinterp/corefcn/bsxfun.cc
--- a/libinterp/corefcn/bsxfun.cc	Wed Mar 21 12:04:19 2018 -0700
+++ b/libinterp/corefcn/bsxfun.cc	Wed Mar 21 10:38:38 2018 -0700
@@ -25,20 +25,20 @@
 #  include "config.h"
 #endif
 
+#include <list>
 #include <string>
 #include <vector>
-#include <list>
 
 #include "lo-mappers.h"
 
-#include "oct-map.h"
 #include "defun.h"
 #include "interpreter.h"
-#include "parse.h"
-#include "variables.h"
+#include "oct-map.h"
 #include "ov-colon.h"
+#include "ov-fcn-handle.h"
+#include "parse.h"
 #include "unwind-prot.h"
-#include "ov-fcn-handle.h"
+#include "variables.h"
 
 // Optimized bsxfun operations
 enum bsxfun_builtin_op
@@ -87,6 +87,7 @@
   for (int i = 0; i < bsxfun_num_builtin_ops; i++)
     if (name == bsxfun_builtin_names[i])
       return static_cast<bsxfun_builtin_op> (i);
+
   return bsxfun_builtin_unknown;
 }
 
@@ -495,16 +496,16 @@
                           if (tmp(0).isreal ())
                             {
                               have_FloatNDArray = true;
-                              result_FloatNDArray
-                                = tmp(0).float_array_value ();
+                              result_FloatNDArray =
+                                tmp(0).float_array_value ();
                               result_FloatNDArray.resize (dvc);
                             }
                           else
                             {
-                              have_ComplexNDArray = true;
-                              result_ComplexNDArray =
-                                tmp(0).complex_array_value ();
-                              result_ComplexNDArray.resize (dvc);
+                              have_FloatComplexNDArray = true;
+                              result_FloatComplexNDArray =
+                                tmp(0).float_complex_array_value ();
+                              result_FloatComplexNDArray.resize (dvc);
                             }
                         }
                       else if BSXINIT(boolNDArray, "logical", bool)
@@ -532,44 +533,33 @@
                 {
                   update_index (ra_idx, dvc, i);
 
-                  if (have_FloatNDArray
-                      || have_FloatComplexNDArray)
+                  if (have_NDArray)
                     {
                       if (! tmp(0).isfloat ())
                         {
-                          if (have_FloatNDArray)
-                            {
-                              have_FloatNDArray = false;
-                              C = result_FloatNDArray;
-                            }
-                          else
-                            {
-                              have_FloatComplexNDArray = false;
-                              C = result_FloatComplexNDArray;
-                            }
+                          have_NDArray = false;
+                          C = result_NDArray;
                           C = do_cat_op (C, tmp(0), ra_idx);
                         }
-                      else if (tmp(0).is_double_type ())
+                      else if (tmp(0).isreal ())
+                        result_NDArray.insert (tmp(0).array_value (), ra_idx);
+                      else
                         {
-                          if (tmp(0).iscomplex ()
-                              && have_FloatNDArray)
-                            {
-                              result_ComplexNDArray =
-                                ComplexNDArray (result_FloatNDArray);
-                              result_ComplexNDArray.insert
-                                (tmp(0).complex_array_value (), ra_idx);
-                              have_FloatComplexNDArray = false;
-                              have_ComplexNDArray = true;
-                            }
-                          else
-                            {
-                              result_NDArray =
-                                NDArray (result_FloatNDArray);
-                              result_NDArray.insert
-                                (tmp(0).array_value (), ra_idx);
-                              have_FloatNDArray = false;
-                              have_NDArray = true;
-                            }
+                          result_ComplexNDArray =
+                            ComplexNDArray (result_NDArray);
+                          result_ComplexNDArray.insert
+                            (tmp(0).complex_array_value (), ra_idx);
+                          have_NDArray = false;
+                          have_ComplexNDArray = true;
+                        }
+                    }
+                  else if (have_FloatNDArray)
+                    {
+                      if (! tmp(0).isfloat ())
+                        {
+                          have_FloatNDArray = false;
+                          C = result_FloatNDArray;
+                          C = do_cat_op (C, tmp(0), ra_idx);
                         }
                       else if (tmp(0).isreal ())
                         result_FloatNDArray.insert
@@ -579,33 +569,11 @@
                           result_FloatComplexNDArray =
                             FloatComplexNDArray (result_FloatNDArray);
                           result_FloatComplexNDArray.insert
-                            (tmp(0).float_complex_array_value (),
-                             ra_idx);
+                            (tmp(0).float_complex_array_value (), ra_idx);
                           have_FloatNDArray = false;
                           have_FloatComplexNDArray = true;
                         }
                     }
-                  else if (have_NDArray)
-                    {
-                      if (! tmp(0).isfloat ())
-                        {
-                          have_NDArray = false;
-                          C = result_NDArray;
-                          C = do_cat_op (C, tmp(0), ra_idx);
-                        }
-                      else if (tmp(0).isreal ())
-                        result_NDArray.insert (tmp(0).array_value (),
-                                               ra_idx);
-                      else
-                        {
-                          result_ComplexNDArray =
-                            ComplexNDArray (result_NDArray);
-                          result_ComplexNDArray.insert
-                            (tmp(0).complex_array_value (), ra_idx);
-                          have_NDArray = false;
-                          have_ComplexNDArray = true;
-                        }
-                    }
 
 #define BSXLOOP(T, CLS, EXTRACTOR)                                      \
                   (have_ ## T)                                          \
@@ -621,6 +589,7 @@
                     }
 
                   else if BSXLOOP(ComplexNDArray, "double", complex)
+                  else if BSXLOOP(FloatComplexNDArray, "single", float_complex)
                   else if BSXLOOP(boolNDArray, "logical", bool)
                   else if BSXLOOP(int8NDArray, "int8", int8)
                   else if BSXLOOP(int16NDArray, "int16", int16)
@@ -825,4 +794,11 @@
 %! a .+= [1 2 3];
 %! assert (a, zeros (0, 3));
 
+%!test <*53179>
+%! im = ones (4,4,2) + single (i);
+%! mask = true (4,4);
+%! mask(:,1:2) = false;
+%! r = bsxfun (@times, im, mask);
+%! assert (r(:,:,1), repmat (single ([0, 0, 1+i, 1+i]), [4, 1]));
+
 */
diff -r ba9d37893822 -r bff4a7c7bc39 liboctave/numeric/bsxfun.h
--- a/liboctave/numeric/bsxfun.h	Wed Mar 21 12:04:19 2018 -0700
+++ b/liboctave/numeric/bsxfun.h	Wed Mar 21 10:38:38 2018 -0700
@@ -34,13 +34,13 @@
 
 inline
 bool
-is_valid_bsxfun (const std::string& name, const dim_vector& dx,
-                 const dim_vector& dy)
+is_valid_bsxfun (const std::string& name,
+                 const dim_vector& xdv, const dim_vector& ydv)
 {
-  for (int i = 0; i < std::min (dx.ndims (), dy.ndims ()); i++)
+  for (int i = 0; i < std::min (xdv.ndims (), ydv.ndims ()); i++)
     {
-      octave_idx_type xk = dx(i);
-      octave_idx_type yk = dy(i);
+      octave_idx_type xk = xdv(i);
+      octave_idx_type yk = ydv(i);
       // Check the three conditions for valid bsxfun dims
       if (! ((xk == yk) || (xk == 1 && yk != 1) || (xk != 1 && yk == 1)))
         return false;
@@ -53,25 +53,25 @@
   return true;
 }
 
-// since we can't change the size of the assigned-to matrix, we cannot
-// apply singleton expansion to it, so the conditions to check are
-// different here.
+// For inplace operations the size of the resulting matrix cannot be changed.
+// Therefore we can only apply singleton expansion on the second matrix which
+// alters the conditions to check.
 inline
 bool
-is_valid_inplace_bsxfun (const std::string& name, const dim_vector& dr,
-                         const dim_vector& dx)
+is_valid_inplace_bsxfun (const std::string& name,
+                         const dim_vector& rdv, const dim_vector& xdv)
 {
-  octave_idx_type drl = dr.ndims ();
-  octave_idx_type dxl = dx.ndims ();
-  if (drl < dxl)
+  octave_idx_type r_nd = rdv.ndims ();
+  octave_idx_type x_nd = xdv.ndims ();
+  if (r_nd < x_nd)
     return false;
 
-  for (int i = 0; i < drl; i++)
+  for (int i = 0; i < r_nd; i++)
     {
-      octave_idx_type rk = dr(i);
-      octave_idx_type xk = dx(i);
+      octave_idx_type rk = rdv(i);
+      octave_idx_type xk = xdv(i);
 
-      // Only two valid canditions to check; can't stretch rk
+      // Only two valid conditions to check; can't stretch rk
       if (! ((rk == xk) || (rk != 1 && xk == 1)))
         return false;
     }