googletest icon indicating copy to clipboard operation
googletest copied to clipboard

Enable user-defined templated PrintTo to not be ambiguous

Open MakersF opened this issue 3 years ago • 5 comments

I'm proposing to make the following compile

namespace user {

struct A {};

template<typename T, typename = std::enable_if_t<...>>
void PrintTo(const T& t, std::ostream* os) { ... }
}

testing::PrintToString(user::A{});

which currently fails due to an ambiguous overload with the PrintTo defined in gtest.

Does the feature exist in the most recent commit?

No

Why do we need this feature?

Enable to define PrintTo methods for templates, using SFINAE to constrain them on the right type.

Typical case:

Library authors can define a concept (intended as an set of requirements a type must satisfy) that users of the library need to implement. Given this concept, they might be able to print the type in an effective way. It would be good to enable the library to define a PrintTo that all the types implementing the concept can use.

The possible workarounds are

  1. Create a wrapper type which defines the operator<<. This cannot be used for final types (my current situation). It also requires the user to wrap every object into such wrapper.
  2. Define a PrintTo for each type. The library author not be able to, if the type is created by the user. For the user, this might be very tiresome if there are many types
  3. Use C++20, using a requires clause on the function. Since require clauses are considered more specific than unconstrained templates, this fixes the ambiguous problem. Unfortunately C++20 won't be available in production to many people for quite a while.

See https://stackoverflow.com/questions/25146997/teach-google-test-how-to-print-eigen-matrix for a user struggling with the ambiguity of the template.

Google itself has this problem: protobuf printer is integrated inside google test, I suspect because it's not possible to add it as an external component, since it does concept detection on any type.

Describe the proposal

Resolve the template ambiguity.

It can be done by removing the PrintTo (from https://github.com/google/googletest/blob/1b18723e874b256c1e39378c6774a90701d70f7a/googletest/include/gtest/gtest-printers.h#L438 ), and change the UniversalPrinter to detect whether a valid PrintTo function exists, and call that if so, otherwise call the fallback. Special care needs to be taken because of implicit conversions that would make the PrintTo function selected when currently the fallback is used.

I have managed to get this to compile and pass all the existing tests, and I'm currently testing it against a very large code base to ensure it's a backward compatible change.

I'm looking to upstream the change if there is interest.

Is the feature specific to an operating system, compiler, or build system version?

No

MakersF avatar Nov 17 '21 21:11 MakersF

Perhaps I'm missing something. Could this be done on operator<< instead of PrintTo?

asoffer avatar Nov 22 '21 20:11 asoffer

It's best explained in https://google.github.io/googletest/advanced.html#teaching-googletest-how-to-print-your-values

Sometimes, this might not be an option: your team may consider it bad style to have a << operator for Bar, or Bar may already have a << operator that doesn’t do what you want (and you cannot change it).

In my specific case, I want to be able to provide a PrintTo implementation for Thrift structs.

  1. I'm not in control of the generated code, but I know it follows a specific pattern (and there is support for printing any such generated type)
  2. The generated classes are final
  3. Defining operator<< is dangerous because some data shouldn't end up in logs/error messages.

PrintTo is the right tool in these situation, but due to an implementation detail, defining

template <typename T, typename = std::enalbe_if_t<is_thrift_struct_v<T>>>
void PrintTo(const T& obj, std::ostream* os) { ... }

results in an error due to an ambiguous function existing. I'm proposing to fix it, so that the above works.

I hope this helps clarifying the idea

MakersF avatar Nov 22 '21 23:11 MakersF

Thanks for the explanation, and sorry for the delay. This seems entirely reasonable.

asoffer avatar Dec 07 '21 03:12 asoffer

I am also interested in this feature, and I have an even simpler suggestion:

Why don't we use the tag dispatch trick and change the current default print to template<typename T> void PrintTo(const T& obj, std::ostream* os, SomeSuperType = {})

Then user-provided overloads can declare a sub-type (provided in google test) as the third argument, which makes that overload more specific and thus not ambiguous anymore.

template <typename T, typename = std::enalbe_if_t<...>>
void PrintTo(const T& obj, std::ostream* os, SubType = {}) { ... }

Apart from the new types and that one PrintTo signature, no other change in the main codebase will be necessary.

qc00 avatar Jan 23 '23 21:01 qc00

I ran into this same issue. FWIW, C++20 concepts offer a way around this limitation (credit to StackOverflow user Paul Saunders for identifying this):

namespace user {

struct A {};

template<typename T> requires my_custom_concept
void PrintTo(const T& t, std::ostream* os) { ... }
}

testing::PrintToString(user::A{});

However, it would still be nice to see this fixed for C++17 and earlier.

hwilliams265 avatar Jun 12 '23 22:06 hwilliams265