Re: [eigen] patch to throw std::bad_alloc

[Kenneth Riddile <kfriddile@xxxxxxxxx>]

--- On Tue, 12/16/08, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
> Thanks a lot for your patch. Having never worked with
> assertions I
> actually didn't know how this should be done.
> 
> I committed your patch with some modifications, so if you
> svn up you
> could try it and see if you like it.

It looks good to me.

> Notice that on the linux platform, Eigen will also try to
> use dynamic
> stack allocation in ei_stack_alloc() in Memory.h, which has
> undefined
> behavior in case of stack overflow. Which I guess you would
> frown upon
> ;) so if you want to disable that, just define
> EIGEN_STACK_ALLOCATION_LIMIT to 0.

Heh, I think I will define that 0.  There is no standardized stack overflow exception that I know of.  From what I've read and heard talking to people, such an exception would just promote bad habits, since it should be possible to prevent stack overflows at coding-time by simply avoiding situations where they could occur.  That means not using alloca() in favor of heap allocation.  My machine's man pages even have this to say about it:

"The alloca() function is machine and compiler dependent. On many systems its implementation is buggy. Its use is discouraged."

I know you said you're only using it on *nix platforms, which is good, because the Microsoft alloca() behaves a lot differently than the GCC implementation in that it DOES throw an implementation-defined exception.  Something to think about, but, if I can disable it, then that's good enough for me.

> 
> Now the comments...
> 
> >+#include <new>
> 
> I moved that to the Eigen/Core header. The place were you
> put it is
> actually included inside namespace Eigen!

Ah, yes, that was unintentional and simply a result of different coding styles.  I'm more used to wrapping the contents of a file in the appropriate namespace blocks instead of wrapping the #include since it simplifies maintenance (reduces unintentional namespace pollution and ensures that moving an #include doesn't subtly alter the meaning of that file's contents).

> Also we only include that if exceptions are enabled. I
> introduced
> EIGEN_EXCEPTIONS.
> 
> > inline T* ei_aligned_malloc(size_t size)
> > {
> 
> Here I made some modifications: fix the warning
> "unused variable
> failed" when exceptions are disabled, avoid an if(),
> etc.

Sounds good.

> >+      throw std::bad_alloc;
> 
> This failed to compile, I had to add parentheses. OK? (i
> don't have
> any experience)

Ya, that was a typo.  std::bad_alloc is a class like any other, so the unnamed temporary there needs parentheses.

Thanks,
Kenny Riddile

---