[boost] Conformance issues in Boost.Icl

Subject: [boost] Conformance issues in Boost.Icl
From: Doug Gregor (doug.gregor_at_[hidden])
Date: 2011-03-05 16:35:52

Hi all,

I've been going through the Boost.Icl test failures for Clang, and
there are few conformance issues that should be addressed on the Boost
side.

The main issue is the test fastest_set_icl_set.cpp, which has some
declaration-ordering issues with templates that only Clang seems to
diagnose (I've verified that Clang is right). The basic problem, which
is repeated in a few places, is that Boost.Icl adds operators and
operator templates into namespace boost::icl, e.g., from
<boost/icl/concept/element_associator.hpp>:

namespace boost{namespace icl
{
  template <class Type>
  inline typename enable_if<is_associative_element_container<Type>, Type>::type
  operator + (Type object, const Type& operand)
  {
      return object += operand;
  }
} }

Now, this operator + can be used with any associative element
container, which includes std::set, but only in a context where normal
name lookup will find the operator. ADL doesn't help, since the
associated namespaces of std::set don't include boost::icl [1].

In the test file fastest_set_icl_set.cpp, that use of the operator
comes from the instantiating this macro in <lib/icl/test_laws.hpp>:

#define DEFINE_ASSOCIATIVITY_CHECK_WRT(op_tag, op_sign) \
template<class Type, class TypeB, class TypeC> \
void check_associativity_wrt_##op_tag(const Type& a, const TypeB& b,
const TypeC& c) \
{ \
    Type left  = (a op_sign b) op_sign c; \
    Type right = a op_sign (b op_sign c); \
    BOOST_CHECK(left==right); \
}

which is instantiated withe op_sign=+ in namespace boost::icl, so name
lookup on + looks inside namespace boost::icl and finds the operator+
from earlier.

Except that it doesn't, because <lib/icl/test_laws.hpp> is included
*before* <boost/icl/concept/element_associator.hpp>. So, the operator+
we wanted isn't visible from the template definition, and isn't
visible to ADL during template instantiation, and the code is
ill-formed.

Moving the inclusion of test_laws.hpp earlier fixes this particular
instance of the problem, but there's a more serious one lurking: the
header <boost/icl/concept/element_set.hpp> tends to come very early in
translation, and has this:

template<class Type>
inline typename enable_if<is_element_set<Type>, Type>::type
operator & (Type object, const Type& operand)
{
    return object &= operand;
}

which depends on other operator &= definitions in namespace boost::icl
that tend to come later (including in headers that #include
<boost/icl/concept/element_set.hpp>). I didn't try to detangle these
relations, because it's a header ordering issue that requires more
knowledge of the library than I have. The right fix is probably to
provide forward declarations for these operations in a common header
that always gets included very early.

Another probably-not-significant issue is that the use of template
template parameters whose template parameters have default arguments
(in <lib/icl/test/test_interval_quantifier_shared.hpp>) *might* not be
well-formed in the standard. The standard is fairly silent on the
issue, although the likely resolution of core issue #150 ([3], item
#2) does support this usage, and Clang seems to have been the only
compiler to get this wrong.

Big thanks to Joachim for such an excellent compiler test case :)

  - Doug

[1] Naturally, the template arguments given to set::set might bring
boost::icl into scope, but the test we're talking about using
std::set<int>.
[2] This is a common compatibility issue. See
http://clang.llvm.org/compatibility.html#dep_lookup
[3] http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#150