Possible Operator Usage/Deduction Error when Using REGISTER_STRUCT macros

[fwilmsh]

Describe the bug
It seems I cannot use a custom serialisation operator for std::optional, when the optional is used as a direct struct member.
It works when I wrap a custom type/struct into the std::optional though.

To Reproduce
See code example below.

Expected behavior
Both versions from example compile?

Real (buggy) behavior
It seems during usage of the macro lib tries to use one of its own operators before seeing the custom definition, but I'm guessing here.

Additional context
sdbus-cpp from master, (c6705fa), version contains bug fixes for some enum handling I required earlier.

// Type Helpers for std::optional
#include "sdbus-c++/sdbus-c++.h"
#include <optional>

template<typename T>
struct sdbus::signature_of<std::optional<T>> : public sdbus::signature_of<sdbus::Variant>{};

template<typename T>
inline sdbus::Message& operator<<(sdbus::Message& os, const std::optional<T> maybeValue){
  static_assert(!std::is_same_v<T, bool>);
  if(maybeValue){
    os << sdbus::Variant(maybeValue.value());
  }
  else {
    os << sdbus::Variant(false);
  }
  return os;
}

template<typename T>
inline sdbus::Message& operator>>(sdbus::Message& is, std::optional<T>& maybeValue){
  static_assert(!std::is_same_v<T, bool>);
  sdbus::Variant tmp;
  is >> tmp;
  if(tmp.containsValueOfType<T>()){
    maybeValue = tmp.get<T>();
  } else {
    maybeValue = std::nullopt;
  }
  return is;
}

// This is working fine
struct Foo {
        int member;
};

struct Bar {
    std::optional<Foo> foo;
};

SDBUSCPP_REGISTER_STRUCT(Foo, member);
SDBUSCPP_REGISTER_STRUCT(Bar, foo);

// When using a type for which operators are already provided by the lib itself,
// it seems the template providers up top are not used/found by the compiler.
struct Foobar {
    std::optional<int> foobar;
};

SDBUSCPP_REGISTER_STRUCT(Foobar, foobar);

I'm basically looking for some guidance on this issue. Not really sure if its a bug with the lib or if I'm using it just wrong. Let me know if I can provide more info on this.

[sangelovic]

Hi, it's not dependent on a basic type defined by the lib -- you'll get the same error when you move Foo struct definition from global scope into a namespace.

The serialization and deserialization functions shall be placed in the sdbus namespace. See the example in https://github.com/Kistler-Group/sdbus-cpp/blob/master/docs/using-sdbus-c++.md#extending-sdbus-c-type-system. The lookup should work without problems then.

Let me know if that helped.

[sangelovic]

@fwilmsh Here is the likely explanation to your issue:

To find the correct operator<< implementation, the compiler involves unqualified name lookup (UNL) as well as argument dependent lookup (ADL) when building the set of candidates for overloaded operator resolution:

1. In case of Bar registration, the SDBUSCPP_REGISTER_STRUCT macro calls sdbus::detail::serialize_pack() method which calls operator<< for all struct members. So from the context of sdbus::detail::serialize_pack(), when resolving operator<<() for std::optional<Foo> the compiler finds these candidates:

   • In UNL lookup for overloaded operators (see https://en.cppreference.com/w/cpp/language/unqualified_lookup#Overloaded_operator), it searches sdbus::detail namespace, then sdbus, then global scope. It finds sdbus::operator<<(sdbus::Message&, const Foo&) in sdbus namespace, which has been provided by above SDBUSCPP_REGISTER_STRUCT(Foo, member);. Here the compiler stops visiting upper scopes (the global scope), because it found this one in sdbus. In global scope is your definition of operator<< for std::optional, but is not found by the compiler here.
   • In UNL lookup for overloaded operators, it also searches member overloads -- it finds all member sdbus::Message::operator<<(...) overloads.
   • thanks to the ADL, also the scope of std namespace is searched (because of std::optional parameter), where it finds some overloads. But the scope of std::optional template parameter is also searched, i.e. the scope of Foo (see https://en.cppreference.com/w/cpp/language/adl#Details, bullet point 3 in second paragraph), which is global scope, and in there it finds your implementation of operator<< for std::optional.

   From this list of (quite a number of) operator<< candidates, your implementation is picked -- which is correct and expected.

2. However, in case of Foobar registration, the compiler is again from the context of sdbus::detail::serialize_pack() searching for candidates of operator<<() for std::optional<int>. It finds these:

   • sdbus::operator<<(sdbus::Message&, const Foo&) and sdbus::operator<<(sdbus::Message&, const Bar&) in sdbus namespace, which has been provided by SDBUSCPP_REGISTER_STRUCT macro instantiations for Foo and Bar. Again, since it found some in sdbus namespace, it doesn't check the upper block -- the global scope.
   • all member sdbus::Message::operator<<(...) overloads.
   • thanks to the ADL, also operators in std namespace. But now, since std::optional is upon type int, https://en.cppreference.com/w/cpp/language/adl#Details bullet point 1 in second paragraph holds, which says that for fundamental types no extra namespaces and scopes are searched.

   This means that your operator<< implementation for std::optional<int> in global namespace is not seen by the compiler in this resolution process. So this explains why there is an issue for std::optional<int> but not for std::optional<Foo>.

   You can verify yourself -- if you remove previous SDBUSCPP_REGISTER_STRUCT macros for Foo and Bar, then the compiler will find your implementation for std::optional<int> in the UNL lookup (because if finds nothing in sdbus namespace, so it checks global scope). The compilation will succeed.

So the solution to this is as I said before: place the operator<< and operator>> for your custom types in sdbus namespace and they will be always found -- by the UNL lookup.

Alternatively, you can also place them in the namespace of your custom type (but extending std namespace for this purpose is not recommended), in which case they would be found by the ADL lookup. This however doesn't work for fundamental types -- they are not your custom types.

[sangelovic]

@fwilmsh Can we close the discussion thread?

[fwilmsh]

It seems to work @sangelovic. Thanks for the detailed explanation.
I actually did do a workaround already by moving the operators into library-code itself, but I couldn't figure out why that worked. :)
This clarifies that. Thread can be closed I think.