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

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

In https://reviews.llvm.org/D33059#778617, @zturner wrote:

> 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.


I think it is possible to avoid the explosion of `EXPECT/ASSERT_THAT_<FOO>` macros (I can't think of any right now, but I'm sure something will crop up eventually, which will require specific handling). For example, we could just define a single expect macro, and hide all of the type-specific stuff in the helper function. Then overload resolution should take care of selecting the right behavior.

I'm thinking of something like:

  #define EXPECT(Obj, Matcher) EXPECT_THAT(::llvm::detail::gmock_helper(Obj), Matcher)
  #define ASSERT(Obj, Matcher) ASSERT_THAT(::llvm::detail::gmock_helper(Obj), Matcher)
  
  template<typename T>
  T gmock_helper(T Obj) { return Obj; }
  
  template<typename T>
  ExpectedHolder<T> gmock_helper(Expected<T> Obj) { ... }

This way, we could just write `ASSERT(foo, bar)` always (names are just a suggestion, but I would prefer something shorter than ASSERT_THAT_EXPECTED), and if a type needed special handling, we would just specialize `gmock_helper`. What do you think?

(Disclaimer: I haven't tried to actually implement this, but I don't see a reason why it wouldn't work)


https://reviews.llvm.org/D33059