warnin about 'info' is a bug in eigs-base.cc

[John W. Eaton]

On 29-Aug-2012, Daniel J Sebald wrote:

| There are a string of warnings related to 'info' that I noted a couple 
| posts ago:
| 
| In file included from ../../octave/libinterp/dldfcn/eigs.cc:39:0:
| ../../octave/liboctave/sparse-base-chol.h: In function 'bool 
| make_cholb(SparseMatrix&, SparseMatrix&, ColumnVector&)':
| ../../octave/liboctave/sparse-base-chol.h:56:9: warning: 'info' may be 
| used uninitialized in this function
| ../../octave/liboctave/eigs-base.cc:368:19: note: 'info' was declared here
| [snip]
| 
| Those don't have anything to do with "octave.info".  'info' is a 
| variable here.
| 
| First, I assume the following inclusion in eigs.c is intended:
| 
| #include "eigs-base.cc"
| 
| as I don't see an associated "eigs-base.h" header file.  But it is 
| somewhat unconventional to include C++ code via the header route.
| 
| Anyway, these warnings are actually a bug.  In the constructor for 
| sparse_base_chol_rep is variable "info" defined as a reference:
| 
|      sparse_base_chol_rep (const chol_type& a, octave_idx_type& info,
|                            const bool natural)
|        : count (1), Lsparse (0), Common (), is_pd (false), minor_p (0),
|          perms (), cond (0)
|        {
|          info = init (a, natural, info);
|        }
| 
| but the init member function definition is not a reference, but passed 
| in via the stack and is called 'nargout'.
| 
|      octave_idx_type init (const chol_type& a, bool natural = true,
|                            octave_idx_type nargout = 0);
| 
| So nargout = info as an input variable.  (info is also used as an output 
| variable.)  I'm not sure what nargout stands for--possibly it is in 
| reference to the command outputs, i.e., the usual scenario.
| 
| I do know there is some conditional code in sparse-base-chol.cc:
| 
|    if (is_pd || nargout > 1)
|      {
| 
| So this is a bug in the sense that "info", being uninitialized, could be 
| greater than one (or not) based on what's sitting randomly on the stack.
| 
| I believe what is happening is that the sparse code is doing some work 
| in the background, and if at the point of the conditional test either 
| the sparse matrix is positive definite (is_pd) or a wrong number of 
| output arguments was given in the command (nargout), the background 
| processing will be abandoned.
| 
| Judging from the conditional, the number of arguments can be 0 or 1 and 
| after that there is no further use of nargout.  Therefore, I picked a 
| value of 1 to initialize 'info'.  (See patch.)
| 
| If there is anyone who thinks there should be some way to determine 
| 'nargout' before issuing the constructor in these two cases (i.e., set 
| 'info' to something other than 1), and this is relevant at this point in 
| the code (e.g., there isn't already something ahead of it 
| sanity-checking the number of input and output arguments), please 
| describe.  Right now, nargout seems like something extraneous given it 
| will never be greater than 1 in the cases I've seen; I'm not 100% sure 
| though.  It can be left as is, so long as something valid is passed into 
| the constructor.
| 
| Dan
| 
| ----------------------------------------------------------------------
| diff -r 87f337783d95 liboctave/eigs-base.cc
| --- a/liboctave/eigs-base.cc  Wed Aug 29 10:17:36 2012 -0700
| +++ b/liboctave/eigs-base.cc  Wed Aug 29 17:13:32 2012 -0500
| @@ -365,7 +365,7 @@
|  static bool
|  make_cholb (SparseMatrix& b, SparseMatrix& bt, ColumnVector& permB)
|  {
| -  octave_idx_type info;
| +  octave_idx_type info = 1;
|    SparseCHOL fact (b, info, false);
|  
|    if (fact.P () != 0)
| @@ -403,7 +403,7 @@
|  make_cholb (SparseComplexMatrix& b, SparseComplexMatrix& bt,
|              ColumnVector& permB)
|  {
| -  octave_idx_type info;
| +  octave_idx_type info = 1;
|    SparseComplexCHOL fact (b, info, false);
|  
|    if (fact.P () != 0)

It looks like Jordi made the change to abuse info by overloading it to
also pass nargout:

  http://hg.savannah.gnu.org/hgweb/octave/rev/82d51d6e470c

Jordi, would you like to explain?  I think it would be better to find
another way to fix this problem.  Maybe add another argument that
forces the calculation to proceed even when the arguments are not
positive definite.  I think that would be better than passing
"nargout".  And I'd really rather not abuse the info argument this
way.

BTW, I'm not seeing the warnings.

jwe