-
Notifications
You must be signed in to change notification settings - Fork 10.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GMock does not use PrintTo() to print argument expectations of type QString #1149
Comments
overloading Edit: |
This has been asked many times in the mailing list. This problem is a C++
one and not directly related to gTest.
First thing you need to understand is how operator<< is found by the
compiler. Mainly scope lookup and ADL (
https://en.wikipedia.org/wiki/Argument-dependent_name_lookup)
The operator has to be introduced in the right namespace and at the right
time (before it is used).
The second thing you need to verify is that you are not falling into ODR
violations.
The main point here is that _everyone in the whole binary_ that tries to
debug print that type in gTest must see your definition of operator<<.
If some callers see it and some don't you have ODR violations and undefined
behavior. This UB can appear as if the operator is not called even though
it is there.
The place to add extensions to types is either in the file that declared
the type, or the file that declared the extension point. That ensures that
everyone using that combination can see it. You can't add operator<< for a
type you don't own without risking ODR violations.
But you can add PrintTo overloads inside gTest, because gTest is the place
that declares the that extension.
The best location for local extensions like that is in
gtest/include/gtest/internal/custom. There are empty headers there whose
whole purpose is for local installations to add extra extensions like this.
Go to gtest/include/gtest/internal/custom/gtest-printers.h and add (in the
right namespace, and the right #includes):
void PrintTo(const QString& s, std::ostream* os) { ... }
Now every user of your installation of gTest will agree on how to pretty
print a QString in gTest.
…On Thu, Jul 13, 2017 at 6:17 AM, Knitschi ***@***.***> wrote:
overloading std::string testing::PrintToString(const QString& ) seems to
do the trick.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1149 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ANcRPavNRp8wXibC0M04MqkDtvEVNTihks5sNe7TgaJpZM4OWyE_>
.
|
Thank you for your answer. I played more around with it and found out that I had another test that did not have my So is there no way to get a warning if this happens? Usually the linker warns when multiple definitions for a function exist. Why not here? |
On Thu, Jul 13, 2017 at 1:13 PM, Knitschi ***@***.***> wrote:
Thank you for your answer.
I played more around with it and found out that I had another test that
did not have my PrintTo( const QString& str, ...) declaration available.
Compiling this test seemed to cause the usage of the standard PrintTo().
When I commented this test out, my definition was used. The usage of my
definition also depended on the compilation order and filenames of the
files that contained the tests.
Yes, these are all indications of the undefined behavior.
So is there no way to get a warning if this happens? Usually the linker
warns when multiple definitions for a function exist. Why not here?
Because there are meant to be multiple definitions of this function (ie the
underlying printing function inside testing::internal).
It is template, after all.
The problem is not that there are multiple definitions. It is that the
definitions are different.
This particular problem is does not require diagnostics from the
compiler/linker. It can be hard or expensive to do so.
If you want to read the text in the spec, look up [basic.def.odr].
The relevant parts are:
<snip>
Every program shall contain exactly one definition of every non-inline
function or variable that is odr-used
in that program; no diagnostic required.
<snip>
There can be more than one definition of a class type (Clause 9),
enumeration type (7.2), inline function with external linkage (7.1.2),
class template (Clause 14), non-static function template (14.5.6), static
data member of a class template (14.5.1.3), member function of a class
template (14.5.1.1), or template specialization for which some template
parameters are not specified (14.7, 14.5.5) in a program provided that each
definition appears in a different translation unit, and provided the
definitions satisfy the following requirements. <snip> If the definitions
of D do not satisfy these requirements, then the behavior is undefined.
Changing the headers of an external library does not seem to be a clean
solution.
You are trying to modify the API of an external library. The best place to
do it is in the header for the external library.
Note that the /custom/ directory in gTest was added explicitly to support
local extensions, so it is not as if you are hacking code that you should
not be touching.
… —
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1149 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ANcRPTwKsxUB7qxdRpS4PJ1Cz-KYZG71ks5sNlBSgaJpZM4OWyE_>
.
|
Thank you again for the elaborate answer. At least after all this trouble I will not forget the Problem and be more cautious in the future. I have not yet experienced such kind of "cross talk" between two compilation units. I guess I never accidently violated the ODR rule before. I am using the hunter-package version of googletest which mainly adds an install target and a cmake config file. I guess I have to figure out now how clients of the package can add a custom You can close this issue if you want. |
@sbenzaquen Thank you, this worked perfectly for printing For anyone else who might be interested, here is my working code (in // ** Custom implementation starts here **
#ifndef GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_
#define GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_
#include <boost/optional.hpp>
namespace boost
{
template <class T>
inline void PrintTo(const optional<T>& o, ::std::ostream* os)
{
*os << (o ? ::testing::PrintToString(o.get()) : "(boost::none)");
}
}
#endif // GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_ |
boost::optional already has the required streaming operators. You should
not need to define your own.
What you want to do is just #include <boost/optional/optional_io.hpp> in
custom/gtest-printers.h.
That way all users of gTest in your installation will implicitly have
access to it.
…On Tue, Dec 19, 2017 at 1:02 PM, bobkocisko ***@***.***> wrote:
@sbenzaquen <https://github.com/sbenzaquen> Thank you, this worked
perfectly for printing boost::optional<T> (after countless failed
attempts to define this elsewhere).
For anyone else who might be interested, here is my working code (in
custom/gtest-printers.h). It makes all the difference in the world to see
the actual printout in the error messages.
// ** Custom implementation starts here **
#ifndef GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_
#define GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_
#include <boost/optional.hpp>
namespace boost
{template <class T>inline void PrintTo(const optional<T>& o, ::std::ostream* os)
{
*os << (o ? ::testing::PrintToString(o.get()) : "(boost::none)");
}
}
#endif // GTEST_INCLUDE_GTEST_INTERNAL_CUSTOM_GTEST_PRINTERS_H_
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1149 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ANcRPUZQi9v7GE24VY9_eLTYWw47M2O_ks5tB_owgaJpZM4OWyE_>
.
|
Thanks, good to know. I tried it and it works too (yay!) but I actually prefer the implementation I posted because it does a nicer job formatting |
I have been cleaning up older and inactive GitHub googletest issues. You may see an issue "closed" if it appears to be inactive/abandoned |
2071: Add stream operator for miral::Window r=AlanGriffiths a=wmww This makes window's correctly appear in test failures, and may have other uses. google/googletest#1149 explains why the stream operators really need to be declared in the same place as the class. I tried a number of other ways, and this is the only one I found to be successful. Co-authored-by: William Wold <[email protected]>
2071: Add stream operator for miral::Window r=AlanGriffiths a=wmww This makes window's correctly appear in test failures, and may have other uses. google/googletest#1149 explains why the stream operators really need to be declared in the same place as the class. I tried a number of other ways, and this is the only one I found to be successful. Co-authored-by: William Wold <[email protected]>
It looks like some other print function for arrays is used instead:
here is a longer discussion of the problem:
https://groups.google.com/forum/#!topic/googletestframework/FMyzItAJ5KE
The text was updated successfully, but these errors were encountered: