CubicalComplex operators not found when same operators are already defined in DGtal namespace.

[rolanddenis]

When including a header that defines the operator- as non-member function in the DGtal namespace, the same operator- that applies on CubicalComplex and that is defined in the DGtal::functions namespace is not found by the Clang compiler (no problem with GNU compiler) when compiling testCubicalComplex.cpp.

This compilation error arises in CubicalComplex.ih:

using namespace ::DGtal::functions;
CubicalComplex cl_star_S = closure( star   ( S, hintOpen   ), hintClosed );
CubicalComplex star_cl_S = star   ( closure( S, hintClosed ), hintOpen   );

// Calls a specializable difference of sets operation.
return cl_star_S - star_cl_S;

when compiling PR #1345 due to the declaration of PointVector operators in the DGtal namespace. It can also be reproduced by including the (unnecessary) header DGtal/dec/KForm.h at the beginning of testCubicalComplex.cpp (it also defines an operator- in the DGtal namespace).

It seems to come from the name lookup rules that are not completely clear for me. It seems that during the argument-dependent lookup step (see https://en.cppreference.com/w/cpp/language/adl), if a function with same name is found (argument type doesn't matter at this step) in the namespace that contains the arguments of the operator (aka DGtal), then the lookup stage is (partly) aborted and using directive (using namespace ...) are ignored.
So, when no operator- is found in DGtal namespace, then the using directive is considered and the DGtal::functions::operator- is found. But if an operator- is found in DGtal namespace (like with the PointVector related PR or when including DGtal/dec/KForm.h), then the DGtal::functions namespace is ignored and the overload resolution fails since the found function cannot take CubicalComplex as parameters.

I don't know if the compiler should be able to found the right operators because of the friend declarations in CubicalComplex... (is this a bug of Clang ?)

An easy fix (but only for this specific lines of code) is to replace the using directive by the using declaration:

using ::DGtal::functions::operator-;

that would bring the right operator- at the block scope (instead of the namespace scope) and that would then be considered during the unqualified name lookup stage.

Otherwise, it seems to be recommended to put operators in the same namespace as its arguments but it may implies many modifications.

Another solution would be to add using declarations for each operator after their forward declarations in CubicalComplex.h, like:

using functions::operator+;
using functions::operator-;
using functions::operator*;
...

Apart of this, there is several dependency issues in CubicalComplex.h and CubicalComplexFunctions.h:

• the operator- defined in CubicalComplexFunction.h is needed in CubicalComplex.ih but including CubicalComplex.h alone doesn't lead to include CubicalComplexFunction.h
• including CubicalComplex.h and CubicalComplexFunction.h alone doesn't compile since CubicalComplexFunction.ih use SpaceND and HyperRectDomain that are not included.

[JacquesOlivierLachaud]

I am a bit surprised that the compiler does not do a correct name lookup for operator-. As I see in CubicalComplex.h and CubicalComplexFunctions.h, operator- expect CubicalComplex<T1,T2>.
And I think for PointVector operator- also expects PointVector.
Is it a question of namespace lookup ? (ie DGtal vs DGtal::functions)

[rolanddenis]

Yes, from what I've read, I think that's because the operators are in a sub-namespace and since the compiler already founds corresponding functions in DGtal namespace using "argument-dependent lookup", it doesn't look further...

[JacquesOlivierLachaud]

Your fix seems correct for CubicalComplex.ih
Otherwise, I think we can say in the doc that the user must also include CubicalComplexFunctions if he wants to use the operators.

[rolanddenis]

Ok, we should include CubicalComplexFunctions in CubicalComplex.ih ?

[dcoeurjo]

ping @JacquesOlivierLachaud

[JacquesOlivierLachaud]

Perhaps include it in CubicalComplex.h instead to expose the fact that they come together.

[JacquesOlivierLachaud]

By the way, should we put DGtal::operator+( PointVector... ) and others in DGtal::functions ?

[rolanddenis]

We will then have the same issue: if a non-related operator+ is defined elsewhere, it may lead the compiler to ignore our DGtal::functions::operator+. It can be solve by adding using functions::operator+ when declaring the related class but if they should always be shipped together (class and operators), it seems to me more logical to have both in the same namespace.

[JacquesOlivierLachaud]

So we should put operator+( CubicalComplex< >... ) in DGtal namespace (and not DGtal::functions) and include CubicalComplexFunctions in CubicalComplex.h

[JacquesOlivierLachaud]

ping @rolanddenis
Is the solution above ok according to you ? (putting all in namespace DGtal).

[rolanddenis]

Yep, I agree !

[rolanddenis]

Fixed in #1390, thanks!