[boost] Re: operators.hpp bugs - apologies

From: Daniel Frey (d.frey_at_[hidden])
Date: 2005-05-17 15:58:15

Hi Søren,

I hope you enjoyed your holidays :) I CC'ed you to make sure this won't
get lost.

Søren Lassen wrote:
> OK, here are two examples that compile on most compilers, but not
> Borland  5.5 (are there others that do not accept inline friend
> functions in  namespaces?), as fully fledged C++ programs.
> Example1.cpp:
>     #include <boost/operators.hpp>
>     using namespace boost;
>     class a:addable1<a>{
>        public:
>             a& operator+=(const a&){return *this;};
>        };
> 
>     int main(){   
>     }
> 
> Example2.cpp:
>     #include <boost/operators.hpp>
>     bool addable1;
>     int main(){
>     }
> 
> If you think my proposed solution is usable, I shall be happy to write
> a  version of operators.hpp that implements it - but I shall be away
> for  holidays the next 5 weeks, so it will not be immediately. Feel free
> use  whatever you like from my example in the sandbox vault, if you prefer.

I meanwhile looked at your code (only the part for the above) and there
are some points that bother me:

- Looking at boost/config/*, I can see that there are 4 compilers
affected by BOOST_NO_OPERATORS_IN_NAMESPACE, luckily only older
versions. It seems that all current compilers don't need the
work-around. This means to me that we have a working work-around and all
changes are dangerous as they can break existing platforms. New users on
these platforms can probably live with the inconveniences, ports to
these platforms should IMHO be rare. Users of the old platforms are used
to the pain (I still have to use GCC 2.95.2 (yes, .2!) in the company, I
know what I'm talking about ;).

- The first example is IMHO bad style. Using "using" in such a global
context is explictly asking for trouble. My experience shows that
"using" shall be used rarely and as local as possible, preferable at
function scope or when assembling a class' interface. This might not be
your point in the above example, but I don't think the first one is
really "worth" fixing - YMMV. And obviously it would help to find others
that find your fix worth the trouble.

- You work-around is illegal. You are not allowed to apply a
using-declaration to a template-id (see 7.3.3/5). That given, I won't
apply any fix before the code has been tested with all affected
compilers (anyone?) as even if it works for Borland, chances are that
the other compilers do yell at the illegal code. Or maybe you want to
reconsider writing a fix for Borland-only? Still this doesn't seem worth
the effort since the gain is IMHO quite small.

- The second example you gave above could IMHO be fixed in other ways -
like renaming the classes in the global namespace to e.g.
BOOST_addable1, BOOST_addable2, etc. and provide new, correctly named
classes in boost like the "combined operator classes". Example:

// global namespace
template <class T, class B = ::boost::detail::empty_base>
struct BOOST_incrementable : B
{
  friend T operator++(T& x, int)
  {
    incrementable_type nrv(x);
    ++x;
    return nrv;
  }
private: // The use of this typedef works around a Borland bug
  typedef T incrementable_type;
};

namespace boost
{
  // Forwarder for incrementable
  template <class T, class B = ::boost::detail::empty_base>
  struct incrementable : ::BOOST_incrementable< T, B > {};
}

The BOOST_-prefix should prevent all name clashes (BOOST_* is "reserved
for us anyway, right? ;) and the combined operator classes are known to
work, so adding some more of them (stupid forwarders, but anyway) is far
less intrusive than your patch. Both of your examples above should be
solved by this approach AFAICS. As an added bonus, we could get rid of
all "using"s, which is a good thing as "using" is known to be buggy on
some compilers. Of course this is just an idea that I never tested. If
you or other think it's worth a try, I can prepare a patch for people to
test. Comments?

Regards, Daniel