Re: Bugs blocking the 6.1 release

Am 16. Mai 2020 um 16:38 Uhr schrieb "John W. Eaton":
> On 5/16/20 4:58 AM, "Markus Mützel" wrote:
> > Thanks for the quick fix. I could successfully create the tarball with that change (hg id 178d101fd37d).
> > 
> > Cross-building the stable-octave target with MXE Octave fails with the following error:
> > 
> > /home/osboxes/Documents/Repositories/Octave/mxe-octave-stable/tmp-stable-octave/octave-6.0.1/libinterp/octave-value/ov-fcn-handle.cc: In constructor 'octave::simple_fcn_handle::simple_fcn_handle(const octave_value&, const string&)':
> > /home/osboxes/Documents/Repositories/Octave/mxe-octave-stable/tmp-stable-octave/octave-6.0.1/libinterp/octave-value/ov-fcn-handle.cc:181:28: warning: declaration of 'octave_function* fcn' shadows a parameter [-Wshadow]
> >    181 |           octave_function *fcn = m_fcn.function_value ();
> >        |                            ^~~
> 
> [...]
> 
> I'll check out the warnings.  What version of GCC are you using that 
> produces these warnings?  I don't recall seeing them with GCC 8.
> 
> > /home/osboxes/Documents/Repositories/Octave/mxe-octave-stable/tmp-stable-octave/octave-6.0.1/libinterp/octave-value/ov-fcn-handle.cc: In member function 'virtual bool octave_fcn_handle::load_hdf5(octave_hdf5_id, const char*)':
> > /home/osboxes/Documents/Repositories/Octave/mxe-octave-stable/tmp-stable-octave/octave-6.0.1/libinterp/octave-value/ov-fcn-handle.cc:2640:27: error: cannot bind non-const lvalue reference of type 'octave_hdf5_id&' {aka 'long long int&'} to an rvalue of type 'octave_hdf5_id' {aka 'long long int'}
> >   2640 |       if (afh->load_hdf5 (group_hid, space_hid, type_hid))
> >        |                           ^~~~~~~~~
> > /home/osboxes/Documents/Repositories/Octave/mxe-octave-stable/tmp-stable-octave/octave-6.0.1/libinterp/octave-value/ov-fcn-handle.cc:2022:57: note:   initializing argument 1 of 'bool octave::anonymous_fcn_handle::load_hdf5(octave_hdf5_id&, octave_hdf5_id&, octave_hdf5_id&)'
> >   2022 |   bool anonymous_fcn_handle::load_hdf5 (octave_hdf5_id& group_hid,
> >        |                                         ~~~~~~~~~~~~~~~~^~~~~~~~~
> > make[5]: *** [Makefile:20696: libinterp/octave-value/liboctave_value_la-ov-fcn-handle.lo] Error 1
> > 
> > 
> > Some of those are warnings that I also see for native builds on Ubuntu Linux. But the error about the non-const lvalue seems to be Windows specific. I'm not sure why gcc doesn't mind on Linux though (maybe some standard extension?).
> 
> Yeah, strange.  I'll try a Windows build and see if I can understand why 
> that's failing.

I think I finally understand why this is failing:
For native compilations on my Linux box, hid_t is typedef'ed as int64_t. In the version of the HDF5 headers that is included in MXE Octave, hid_t is typedef'ed as int. Like shown in the error message, octave_hdf5_id is likely typedef'ed as int64_t.
Thus, the types match for native builds on Linux. For Windows MXE builds, the compiler tries to create a temporary rvalue of matching type to call load_hdf5 and fails on binding the non-const lvalue reference to the temporary rvalue.
To be honest, the error message would have been a lot more helpful if it mentioned the type of hid_t...

I used the attached changes to fix that error and to also avoid the shadowed variable warnings.

I'll report back when the cross-compilation has finished and I could test the performance on Windows.

Markus

# HG changeset patch
# User Markus Mützel <[email protected]>
# Date 1590156404 -7200
#      Fri May 22 16:06:44 2020 +0200
# Branch stable
# Node ID 8137dd8461265ff15b70d686b6c1d615e5ca1fd3
# Parent  178d101fd37dd2876b2c95397b0a6575baa1401e
ov-fcn-handle.cc: Fix cross-compilation and shadowing warnings.

diff -r 178d101fd37d -r 8137dd846126 libinterp/octave-value/ov-fcn-handle.cc
--- a/libinterp/octave-value/ov-fcn-handle.cc	Wed Apr 29 14:10:27 2020 -0400
+++ b/libinterp/octave-value/ov-fcn-handle.cc	Fri May 22 16:06:44 2020 +0200
@@ -178,10 +178,10 @@
     {
       if (m_fcn.is_defined ())
         {
-          octave_function *fcn = m_fcn.function_value ();
-
-          if (fcn)
-            m_file = fcn->fcn_file_name ();
+          octave_function *fcn_handle = m_fcn.function_value ();
+
+          if (fcn_handle)
+            m_file = fcn_handle->fcn_file_name ();
         }
     }
 
@@ -1123,10 +1123,10 @@
 
     if (m_fcn.is_defined ())
       {
-        octave_function *fcn = m_fcn.function_value ();
-
-        if (fcn)
-          m_file = fcn->fcn_file_name ();
+        octave_function *fcn_handle = m_fcn.function_value ();
+
+        if (fcn_handle)
+          m_file = fcn_handle->fcn_file_name ();
       }
 
     m_parentage.push_front (name);
@@ -2250,13 +2250,13 @@
     unwind_action act ([&tw] () { tw.pop_scope (); });
 
     int parse_status;
-    octave_value anonymous_fcn_handle
+    octave_value anon_fcn_handle
       = interp.eval_string (fcn_text, true, parse_status);
 
     if (parse_status != 0)
       return false;
 
-    octave_fcn_handle *fh = anonymous_fcn_handle.fcn_handle_value ();
+    octave_fcn_handle *fh = anon_fcn_handle.fcn_handle_value ();
 
     if (! fh)
       return false;
@@ -2557,17 +2557,17 @@
 #if defined (HAVE_HDF5)
 
 #if defined (HAVE_HDF5_18)
-  hid_t group_hid = H5Gopen (loc_id, name_arg, octave_H5P_DEFAULT);
+  octave_hdf5_id group_hid = H5Gopen (loc_id, name_arg, octave_H5P_DEFAULT);
 #else
-  hid_t group_hid = H5Gopen (loc_id, name_arg);
+  octave_hdf5_id group_hid = H5Gopen (loc_id, name_arg);
 #endif
   if (group_hid < 0)
     return false;
 
 #if defined (HAVE_HDF5_18)
-  hid_t data_hid = H5Dopen (group_hid, "nm", octave_H5P_DEFAULT);
+  octave_hdf5_id data_hid = H5Dopen (group_hid, "nm", octave_H5P_DEFAULT);
 #else
-  hid_t data_hid = H5Dopen (group_hid, "nm");
+  octave_hdf5_id data_hid = H5Dopen (group_hid, "nm");
 #endif
 
   if (data_hid < 0)
@@ -2576,8 +2576,8 @@
       return false;
     }
 
-  hid_t type_hid = H5Dget_type (data_hid);
-  hid_t type_class_hid = H5Tget_class (type_hid);
+  octave_hdf5_id type_hid = H5Dget_type (data_hid);
+  octave_hdf5_id type_class_hid = H5Tget_class (type_hid);
 
   if (type_class_hid != H5T_STRING)
     {
@@ -2587,7 +2587,7 @@
       return false;
     }
 
-  hid_t space_hid = H5Dget_space (data_hid);
+  octave_hdf5_id space_hid = H5Dget_space (data_hid);
   hsize_t rank = H5Sget_simple_extent_ndims (space_hid);
 
   if (rank != 0)
@@ -2612,7 +2612,7 @@
   OCTAVE_LOCAL_BUFFER (char, nm_tmp, slen);
 
   // create datatype for (null-terminated) string to read into:
-  hid_t st_id = H5Tcopy (H5T_C_S1);
+  octave_hdf5_id st_id = H5Tcopy (H5T_C_S1);
   H5Tset_size (st_id, slen);
 
   if (H5Dread (data_hid, st_id, octave_H5S_ALL, octave_H5S_ALL,