Re: [Boost-users] Possible bug related to for_each and numberedvectors

From: Aleksey Gurtovoy (agurtovoy_at_[hidden])
Date: 2008-06-20 00:51:02

Hi Martin,

> I stumbled across something very strange related to boost::mpl::for_each and numbered vectors and sets (as opposed to the variadic 
> forms). The program below contains a typelist with 21 (i.e. more than BOOST_MPL_LIMIT_SET_SIZE) classes. This typelist is 
> converted to a typelist containing pointers to those classes. Finally the main program calls for_each with that typelist to print 
> the names of the types. This works without problems on Linux and Windows. But now:
> If you remove the include "boost/mpl/vector/vector30.hpp" everything still compiles fine without warnings under both operating 
> systems. On Linux everything continues to work, whereas under Windows nothing is printed anymore.

It's a bug in the library's diagnostics (or, rather, a lack of such) -- 
please see below.

> for_each does not loop through the typelist for an unknown reason. Everything works again, when reducing the number of classes to 
> 20 (and adjusting the include to set20.hpp).
> From my understanding BOOST_MPL_LIMIT_SET_SIZE and its brothers and sisters should not have any impact on numbered sequences, only 
> on variadic ones.

That's correct.

> But still it looks as if something very strange is happening here.

Indeed. There are several factors at play here:

1. To be able to 'push_back' into a 'vectorN' on a compiler without
   'typeof' support you have to have a 'vectorN+1' definition included.
   If you don't have it included, you will get a compilation error:

    #include "boost/mpl/vector/vector10_c.hpp"
    #include "boost/mpl/push_back.hpp"

    using namespace boost::mpl;

    typedef push_back<
          vector10_c<int,1,2,3,4,5,6,7,8,9,10>
        , int_<11>
        >::type t;

    > test.cpp(9) : error C2039: 'type' : is not a member of 'boost::mpl::push_back<Sequence,T>'
    >    with
    >   [
    >       Sequence=boost::mpl::vector10_c<int,1,2,3,4,5,6,7,8,9,10>,
    >       T=boost::mpl::int_<11>
    >   ]

   ... except when you don't, like you experienced first hand.

2. The reason your code doesn't result in the error above is that
   it doesn't invoke 'push_back' directly -- it supplies it as an
   output operation to the inserter.

   When the inserter does eventually invoke 'push_back' on
   'vector20<...>' (at the last transformation step), the
   invocation is done through the 'apply' metafunction and is
   basically equivalent to this:

     typedef apply< push_back<_1,_2>, vector20<...>, C21 >::type t;

   This, of course, shouldn't make any difference and should produce
   the same error, but it doesn't. Instead, it results in 't' being
   an internal implementation type which has nothing to do with vector
   (nor any other sequence).

   That's where the bug (the absence of proper diagnostics) is.

3. Due to the lack of diagnostics in the previous step,
   'ElementClassesAsPointer' ends up being an typedef to an internal
   type that is not a sequence. On a surface, it seems that passing
   a non-sequence to 'for_each' should still result in a compilation
   error. In fact, 'for_each' and other algorithms/metafunctions in
   the library almost never explicitly check conformace of the
   provided template parameters to their corresponding concepts.

   In absence of the explicit concept comformance verification,
   invocation of a sequence algorithm on a non-sequence type
   is almost guaranteed to be a no-op because of the following piece of
   the 'begin'/'end' specification
   (http://www.boost.org/doc/libs/1_35_0/libs/mpl/doc/refmanual/begin.html):

    [..] If the argument is not a Forward Sequence, returns void_.

   Thus the observed effect of

       boost::mpl::for_each<ElementClassesAsPointer> (*this);

   in your example.

   The absence of concept checks can be argued to be another bug.

I've just checked in a fix for the bug in step #2, and also a number of related
fixes that together greatly reduce the chance of silent diagnoctic failures in
other similar situations -- http://svn.boost.org/trac/boost/changeset/46546.

If you are not compiling against the trunk, it should be safe to apply the diff
to your local Boost version as well.

The proper concept checks is something that will probably have to wait
until C++0x is out in the field.

Thank you for taking time to report this,
--
Aleksey Gurtovoy
MetaCommunications Engineering