[regex] patches for Tru64 and HP-UX
Attached patches for 3 regex library headers for the issues with regex library test concept_check on Tru64/cxx and HP-UX/aCC6 described in: <http://thread.gmane.org/gmane.comp.lib.boost.devel/154587> . concepts.hpp_patch (necessary for Tru64 and VMS) . concept_archetype.hpp_patch (necessary for HP-UX) . regex_traits.hpp_patch (necessary for HP-UX) . 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. 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). . 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. . 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. With patches for concept_archetype.hpp and regex_traits.hpp, regex library test concept_check passes on HP-UX. Also, tr1 library tests std_test_regex and test_regex that were previously failing, now pass. If the patches for concept_archetype.hpp and regex_traits.hpp are acceptable, please, apply them. If they are applied to HEAD only, please, mark regex library test concept_check and tr1 library tests std_test_regex and test_regex expected failures in 1.34 on HP-UX/aCC6 (toolset "acc"). Thanks, Boris
Boris Gubenko wrote:
Will apply.
Will do.
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 (); }
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.
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> >
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@johnmaddock.co.uk> To: <boost@lists.boost.org> Sent: Tuesday, February 27, 2007 5:01 AM Subject: Re: [boost] [regex] patches for Tru64 and HP-UX
Boris Gubenko wrote:
Can you test the attached patch then? If you can assure me that the RW version check is correct (presumably you will up the version number if the issue gets fixed?), and that it has no unfortunate effects on the other Boost regression tests (I've checked it fixes the regex issue, but I don't want to run the full tests on a Testdrive machine), then I'll commit. John. Index: boost/config/stdlib/roguewave.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/config/stdlib/roguewave.hpp,v retrieving revision 1.15 diff -u -r1.15 roguewave.hpp --- boost/config/stdlib/roguewave.hpp 19 Oct 2005 16:38:40 -0000 1.15 +++ boost/config/stdlib/roguewave.hpp 28 Feb 2007 10:09:12 -0000 @@ -125,3 +125,20 @@ #if !defined(_RWSTD_LONG_LONG) && defined(BOOST_HAS_LONG_LONG) # undef BOOST_HAS_LONG_LONG #endif + +#if (BOOST_RWSTD_VER <= 0x02020100) && (BOOST_RWSTD_VER >= 0x02000000) && defined(__EDG_VERSION__) +// +// Work around a bug in RW 2.0 lib: +// +#include <iterator> +namespace std{ + template <class _Iterator> + inline typename iterator_traits<_Iterator>::iterator_category + __iterator_category (const _Iterator&) + { + typedef typename iterator_traits<_Iterator>::iterator_category _IterCategory; + return _IterCategory (); + } +} +#endif +
John Maddock wrote:
Can you test the attached patch then?
I've started full tests with *modified* patch: see below.
No, we won't. We don't change RW version number, on any platform, and if we start doing it now, it can break customer code expecting exact comparison to work. All we know about this RW bug is that: . it is not present in 0x0203 (RW library on Tru64 and AlphaVMS) . it is present in 0x02020100 (RW library on HP-UX) . it is not present in 0x03000000 (RW library on iVMS) Tracking the range of RW libraries having this bug would be difficult and we cannot do it without the help from Rogue Wave Software, Inc. Also, the RW version numbering is not always consistent: for example, the library with _RWSTD_VER == 0x0203 is rw libstd v2.0 while the library with _RWSTD_VER == 0x02020100 is rw libstd v2.2. For these reasons, I propose to restrict RW version check to a single library we know for sure has this bug: #if (BOOST_RWSTD_VER == 0x02020100) && defined(__EDG_VERSION__) This is the condition I'm running the tests with now. You've applied the patch for regex_traits.hpp (thank you!), but, unfortunately, you modified the __HP_aCC version check (in two places) so it is still broken: . the original code: !BOOST_WORKAROUND(__HP_aCC, BOOST_TESTED_AT(55500)) . in my patch: !BOOST_WORKAROUND(__HP_aCC, < 60000) . in your patch: !BOOST_WORKAROUND(__HP_aCC, BOOST_TESTED_AT(60000)) The condition should be either: "!BOOST_WORKAROUND(__HP_aCC, < 60000)" or "BOOST_WORKAROUND(__HP_aCC, >= 60000)". Thanks, Boris ----- Original Message ----- From: "John Maddock" <john@johnmaddock.co.uk> To: <boost@lists.boost.org> Sent: Wednesday, February 28, 2007 5:12 AM Subject: Re: [boost] [regex] patches for Tru64 and HP-UX
John Maddock wrote:
Fixed now [...]
Thank you!
I don't have cvs commit rights and I'd appreciate having them. But, as I said in the reply to the release manager, "as a newcomer, I'd be reluctant to make any changes to the rc branch at this point in the development cycle". So, for 1.34, I'll continue to need help from others. But I do think I should have the cvs commit rights, to use them for 1.34.1 and beyond. If nothing else, it will make other people's life easier :-) Thanks again, Boris
Boris Gubenko wrote:
A common tactic for platform focused maintainers with commit access is to make a patch, send it to the list with an 'Ok to commit' query and when the library developer says 'yes' the platform maintainer can just commit. For things that are pretty obviously ok this is easier on the library maintainers. So, I'd suggest you send you sourceforge id to the boost-owner list and we'll put you in now. Jeff
Jeff Garland wrote:
Done. Thanks, Boris ----- Original Message ----- From: "Jeff Garland" <jeff@crystalclearsoftware.com> To: <boost@lists.boost.org> Cc: "Boris Gubenko" <Boris.Gubenko@hp.com> Sent: Wednesday, February 28, 2007 1:12 PM Subject: Re: [boost] [regex] patches for Tru64 and HP-UX
John Maddock wrote:
Unfortunately, the patch broke some tests, in particular, 9 tr1 library tests. This is because roguewave.hpp #include's <iterator>. Just inclusion of <iterator> makes the tests fail. I did not analyze the failures, I guess, the tests don't expect <iterator> to be included. I think, we have 3 options: 1) put your fix in some regex library header which includes <iterator> anyway. 2) put my original fix which does not require any headers in roguewave.hpp 3) do nothing: mark tests affected by this RW bug expected failures and wait for the compiler kit with the fix (for the reference, this RW bug is tracked internally as CR JAGag33243). I'm running the tests with 2) now, to verify that it indeed is a viable solution. Thanks, Boris
Boris Gubenko wrote:
Oh shucks, I forgot about that, try: # ifndef BOOST_TR1_NO_RECURSION # define BOOST_TR1_NO_RECURSION # define BOOST_CONFIG_NO_ITERATOR_RECURSION # endif # include <iterator> # ifdef BOOST_CONFIG_NO_ITERATOR_RECURSION # undef BOOST_TR1_NO_RECURSION # undef BOOST_CONFIG_NO_ITERATOR_RECURSION # endif In any case this is too late for 1.34 now whatever. Thanks, John.
John Maddock wrote:
It does not help. I did not run the full tests, just tr1 library tests. They failed as before.
In any case this is too late for 1.34 now whatever.
Indeed. For 1.34, the tests affected by this RW bug should be made known failures. It may happen automatically during Volodya's cleanup for 1.34, assuming that he does it for all the platforms, not just release platforms (this is something I'm not sure about). Boris Gubenko wrote:
2) fixes the problem without introducing any regression. The results are not on the web yet, I just checked. Do you have any suggestion on how to proceed with this issue beyond 1.34? Thanks for your help, Boris
participants (3)
-
Boris Gubenko -
Jeff Garland -
John Maddock