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

[Daniel J Sebald]

On 08/30/2012 08:43 AM, Jordi Gutiérrez Hermoso wrote:
> On 29 August 2012 18:47, Daniel J Sebald<address@hidden>  wrote:
> > 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.
>
> Yes, there is nothing wrong with #including .cc files. This is quite
> common when they contain template definitions, as is the case here.

But there are more than just declarations in eigs-base.cc.  The 
following are actual code:

static bool
vector_product (multiple input formats)
static bool
make_cholb (multiple input formats)
octave_idx_type
EigsComplexNonSymmetricFunc (EigsComplexFunc fun, octave_idx_type n,

several others.  Other than class definitions, there is just about 
everything in this file (templates, static functions, non-static 
functions, some extern definitions).  eigs-base.cc can only be included 
in one file, otherwise there would be linking duplications.  Given such 
a mix of items, it seems odd to include eigs-base.cc that way.

One other thing is that some of these templates are rather big.  With a 
template that means a lot of code is replicated with just subtle changes 
in input variables.  Is that correct?


> On 29 August 2012 19:26, John W. Eaton<address@hidden>  wrote:
> > 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.
>
> Well, the "right" way would be to change the API all over the place to
> pass an extra argument. I started doing that, and it turned into such
> a horrible mess that I decided to use info instead, since its input
> didn't seem to be used for anything.

Perhaps if nargout is removed, as John suggested, there won't be a 
change in the API.  In any case, it feels to me this file should be 
broken into a header file and code file.


> The sparse code is quite tangled, not to mention copy-pasted, so
> whatever change you make, you have to copy across all of the other
> copy-pasted parts. I got fed up and opted instead for the change that
> required the least number of lines of code to change.
>
> I guessssss that I shooooouuld instead refactor and untangle the
> sparse code. Do you insist I do that? It seems like a stupid
> refactoring effort, but I guess we'll have to do it sooner or later.

I does seem rather tangled, but I'd classify it more as bloated.  The 
functions are easy to follow, it just seems that simplification could 
have been done to reduce C code and ultimately compiled code.

Dan