9

Consider a legacy class template with overloaded addition operators += and +

template<class T>
class X
{
public:
    X() = default;
    /* implicict */ X(T v): val(v) {}

    X<T>& operator+=(X<T> const& rhs)       { val += rhs.val; return *this; }
    X<T>  operator+ (X<T> const& rhs) const { return X<T>(*this) += rhs;    } 

private:
    T val;
};

Upon code review, it is observed that + is implementable in terms of +=, so why not make it a non-member (and have guaranteed symmetry for left and right arguments)?

template<class T>
class X
{
public:
    X() = default;
    /* implicit */ X(T v): val(v) {}

    X<T>& operator+=(X<T> const& rhs)       { val += rhs.val; return *this; }

private:
    T val;
}; 

template<class T>
X<T> operator+(X<T> const& lhs, X<T> const& rhs)
{ 
    return X<T>(lhs) += rhs; 
}

It look safe enough, because all valid expression using + and += retain their original semantic meaning.

Question: can the refactoring of operator+ from a member function into a non-member function break any code?

Definition of breakage (worst to best)

  • new code will compile that did not compile under the old scenario
  • old code will not compile that did compile under the old scenario
  • new code will silently call a different operator+ (from base class or associated namespace dragged in through ADL)
TemplateRex
  • 69,038
  • 19
  • 164
  • 304
  • "Breakage" as in "client code won't compile" or as in "ABI incompatible"? – Oliver Charlesworth Sep 28 '14 at 20:11
  • Short answer: Yes, of course it can. But it's not very likely to... – Mats Petersson Sep 28 '14 at 20:12
  • @MatsPetersson I haven't been able to come up with a concrete example – TemplateRex Sep 28 '14 at 20:14
  • @OliverCharlesworth updated, can it also break ABI compat? – TemplateRex Sep 28 '14 at 20:15
  • It can only break ABI-compat if it was exported by one module and imported by a different one (breaking dynamic linking). Unlikely, as it was an inline-function, and a template-specialization at that. Aside: Your free function should really be `inline`. – Deduplicator Sep 28 '14 at 20:20
  • I agree with Deduplicator, I can't think of a case where it would break unless we're not recompiling something, and that's not going to happen (subject to valid dependency resolution in the build-system) for a template-inline function. – Mats Petersson Sep 28 '14 at 20:22
  • @Deduplicator I'm not looking at optimizations, the `inline` is not required here. In any case, for argument's sake, assume the code is all header-only (updated in the question) – TemplateRex Sep 28 '14 at 20:23
  • Far as I see, only explicit calls to the overloaded operator would be broken, and the breakage is purely compile-time. Excluding the extremely unlikely possibility I mentioned above, for which I think you need explicit extern-template-declarations and in the implementation-file explicit instantiations. () If it's pure header-only, it reduces to the API-breakage atompile-time. – Deduplicator Sep 28 '14 at 20:28
  • @Deduplicator `inline` is redundant; template functions implicitly instantiated have implicit inline linkage. – cdhowie Sep 28 '14 at 20:48
  • @cdhowie: Do you know where it says so? Because I could not find it... – Deduplicator Sep 28 '14 at 20:57
  • 1
    @Deduplicator In modern C/C++, `inline` is primarily used not to indicate that the function should be inlined (the standard does not require compilers to honor `inline` in this way) but rather to indicate that the ODR does not apply (the function is allowed to be defined in multiple translation units without error). Since implicit instantiations of templates are already exempt from the ODR, `inline` doesn't really accomplish anything. They are *not the same thing* but they have *the same effect* (again, with regard to *implicit* instantiations). – cdhowie Sep 28 '14 at 21:05
  • 1
    Well the obvious answer is yes, it can break things at compile-time if you explicitly refer to `+` as a member function somewhere: `X z = x.operator+(y);` But this is not related to ADL, and also extremely unlikely to encounter that. – misberner Sep 29 '14 at 00:18

1 Answers1

3

Summary

The answer is, yes, there will always be breakage. The essential ingredient is that function template argument deduction does not consider implicit conversions. We consider three scenarios, covering the three syntactic forms that an overloaded operator can take.

Here we use an implicit constructor inside X<T> itself. But even if we made that constructor explicit, users could add to the namespace of X<T> a class C<T> that contains an implicit conversion of the form operator X<T>() const. The scenarios below would continue to hold in that case.

A non-member friend function breaks the least in the sense that it will allow lhs argument implicit conversions that would not compile for a class template's member function. The non-member function template breaks the implicit conversion on rhs arguments.

Class template's member function

template<class T>
class X
{
public:
    /* implicit */ X(T val) { /* bla */ }
//...
    X<T> operator+(X<T> const& rhs) { /* bla */ }
//...
};

This code will allow expression like

T t;
X<T> x;
x + t;  // OK, implicit conversion on non-deduced rhs
t + x;  // ERROR, no implicit conversion on deduced this pointer

Non-member friend function

template<class T>
class X
{
public:
    /* implicit */ X(T val) { /* bla */ }
//...
    friend 
    X<T> operator+(X<T> const& lhs, X<T> const& rhs) { /* bla */ }
//...
};

Since the friend function is a not a template, no argument deduction takes place and both the lhs and rhs argument consider implicit conversions

T t;
X<T> x;
x + t;  // OK, implicit conversion on rhs
t + x;  // OK, implicit conversion on lhs

Non-member function template

template<class T>
class X
{
public:
    /* implicit */ X(T val) { /* bla */ }
//...
};

template<class T> 
X<T> operator+(X<T> const& lhs, X<T> const& rhs) { /* bla */ }

In this case, both the lhs and rhs arguments undergo argument deduction, and neither takes implicit conversions into account:

T t;
X<T> x;
x + t;  // ERROR, no implicit conversion on rhs
t + x;  // ERROR, no implicit conversion on lhs
TemplateRex
  • 69,038
  • 19
  • 164
  • 304