Re: [boost] [regex] patches for Tru64 and HP-UX

From: Boris Gubenko (Boris.Gubenko_at_[hidden])
Date: 2007-02-27 17:02:34

John,

thanks for the reply and for the willingness to apply the patches.

> I'm a bit dubious about applying this for a few reasons:
> [...]

I understand. Let me try to address your concerns. First, since this
is a Rogue Library issue, perhaps, boost/concept_archetype.hpp is not
the right header for the workaround. I think thatthe proper header
would be: boost/config/stdlib/roguewave.hpp . Sorry about that.

Second, regarding the claim that this is a bug in the RW 2.2 library,
I agree: this is a bug. I (re)read 24 - Iterators library [lib.iterators]
and looked at your bidirectional_iterator_archetype and I'm pretty sure
that this is a RW bug.

In my first mail on the topic, to illustrate the problem, I used the
following program derived from regex library test concept_check.cpp :

#include <boost/regex/concepts.hpp>
void f() {
  boost::bidirectional_iterator_archetype<
    boost::BoostRegexConcept<
      boost::basic_regex<char, boost::c_regex_traits<char>
    >
  >::value_type> x;
  std::string(x, x);
}

x.cpp below is a reduced reproducer which does not include boost headers:

x.cpp
-----
#include <iterator>
#include <string>

template <class T>
struct bidirectional_iterator_archetype
{
  typedef bidirectional_iterator_archetype self;
  typedef std::bidirectional_iterator_tag iterator_category;
  typedef T value_type;
  typedef const T& reference;
  typedef T* pointer;
  typedef std::ptrdiff_t difference_type;
  self& operator=(const self&);
  bool operator==(const self&) const;
  bool operator!=(const self&) const;
  reference operator*() const;
  self& operator++();
  self operator++(int);
  self& operator--();
  self operator--(int);
};

void f() {
  bidirectional_iterator_archetype<char> x;
  std::string(x, x);
}

In x.cpp above, bidirectional_iterator_archetype satisfies all the
requirements in 24.1.1 - Input iterators [lib.input.iterators] so
there is no reason std::string cannot be constructed from it. Still,
the program does not compile on HP-UX.

So, this is a bug in RW 2.2. The thing is that this bug is fixed in
RW 3.0 (the library on iVMS) by using exactly the same technique you
showed in your mail: "FYI this normally gets implemented using something
like: ...".

We'll fix RW 2.2, but the fix will be available only with the future
compiler update. In the meantime, would you agree to apply workaround,
presumably, to boost/config/stdlib/roguewave.hpp , so that users can use
the functionality which is currently broken because of this bug before
the compiler kit with the fix is available?

When we know the exact aCC6 version corresponding to the library with the
fix, we can conditionalize workaround based on the compiler version so that
it applies only when needed.

If you think that applying a workaround is not a good idea, would it be
possible to mark affected tests expected failures, in both RC and the
HEAD?

Thanks again,
  Boris

----- Original Message ----- 
From: "John Maddock" <john_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Tuesday, February 27, 2007 5:01 AM
Subject: Re: [boost] [regex] patches for Tru64 and HP-UX

> Boris Gubenko wrote:
>>> . The patch for concepts.hpp adds char_type to the explicit
>>>  specialization of template std::char_traits on type
>>>  boost::char_architype. char_type is necessary for the platforms
>>>  using Rogue Wave V2.0 (Tru64 and AlphaVMS) and RW V3.0 (iVMS).
>>>
>>>  While it is not necessary for HP-UX/aCC6 using RW V2.2, it does not
>>>  break it. Adding char_type also does not break compilation on HP-UX
>>>  with Gnu libstdc++ and STLport.
>>>
>>>  In the patch, char_type is conditionalized as the following:
>>>
>>>   template<> struct char_traits<boost::char_architype>{
>>> #if defined(__DECCXX) && BOOST_WORKAROUND(_RWSTD_VER, >= 0x0203)
>>>     typedef boost::char_architype char_type;
>>> #endif
>>>   };
>>>
>>>  so it affects only Tru64, AlphaVMS and iVMS. Actually, I don't think
>>>  that any conditionalization is necessary because adding this typedef
>>>  does not seem to do any harm.
>
> Will apply.
>
>>>  If this patch is applied, regex library test concept_check can be
>>>  removed from explicit-failures-markup.xml for toolset
>>>  "hp_cxx-71_006_tru64" (this is release platform).
>
> Will do.
>
>>> . The patch for concept_archetype.hpp adds overload for
>>>  std::__iterator_category returning __unknown_iterator_tag.
>>>
>>>  This overload allows std::basic_string to be constructed, as John
>>>  Maddock put it in:
>>>
>>>
>>> <http://archives.free.net.ph/message/20060824.111121.1b255f03.en.html>
>>>
>>>  from "iterator types it doesn't know about".
>>>
>>>  This patch is necessary for RW V2.2. I've conditionalized it with:
>>>
>>> #if defined(__HP_aCC) && BOOST_WORKAROUND(_RWSTD_VER, == 0x02020100)
>>>
>>>  but I think, that 'defined(__HP_aCC)' can be omitted because this
>>>  is a pure Rogue Wave library issue. I conditionalized it the way I
>>>  did out of fear of breaking other platforms using the RW library
>>>  that I cannot test the patch on.
>
> I'm a bit dubious about applying this for a few reasons:
>
> 1) It's not my library.
> 2) There is no requirement in the std that iterators that are classes derive
> from std::iterator - only that iterator_traits "works" with them.  That
> could be via specialisation of iterator_traits or by providing the necessary
> member typedefs so that the primary template definition can do it's thing
> (this is what the concept lib does).  So IMO this is a bug in the RW 2.2
> lib.
> 3) Given (2) this is something we might actually want to know about?
>
> FYI this normally gets implemented using something like:
>
> template <class _Iterator>
> inline typename iterator_traits<_Iterator>::iterator_category
> __iterator_category (const _Iterator&)
> {
>    typedef typename iterator_traits<_Iterator>::iterator_category
> _IterCategory;
>    return _IterCategory ();
> }
>
>>> . The patch for regex_traits.hpp changes aCC - specific
>>>  conditionalization which is wrong for aCC6. I have no idea why this
>>>  conditionalization was necessary in the first place, but I did not
>>>  remove it: I just restricted it to pre-aCC6 compilers.
>
> This was already applied to HEAD, but I'll apply to the branch as well now
> that it's had time to shake out any issues.
>
> Thanks for working on this,
>
> John.
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
>
>