[PATCH] D33059: Create an shared include directory for gtest helper functions

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 12 19:03:50 PDT 2017


zturner added a comment.

In https://reviews.llvm.org/D33059#778601, @MatzeB wrote:

> about the directories: 'meh'. I'm fine with whatever works.
>
> - Why is a patch that claims to create a shared include directory actually changing so many unit tests?


I felt like we needed a motivating example of why this effort is useful.  The error macros were copy/pasted amongst many different files, so it was the perfect candidate.

> - `EXPECT_THAT_ERROR(x, Succeeded())` seems bad. SHouldn't this rather be `EXPECT_THAT(x, succeeded())` to look more natural?

You're right, it is bad.  But unfortunately there is not a good alternative.  `Error` is a move only type, and gmock a) can't handle move only types, and b) can't exhibits undefined behavior if you change the state of an object you're matching inside of a matcher.  Because of that, we have to convert the `Error` into something else by basically consuming its failure/success status and error messge and storing them in a different type, and then matching against that type.  So we need the macro to convert the `Error / Expected` into the type we actually match against.

It's not ideal, but it seems like the best we can do for this one specific class.


https://reviews.llvm.org/D33059





More information about the llvm-commits mailing list