[PATCH] D34405: [Testing/Support] Remove the const_cast in TakeExpected

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 21 03:35:06 PDT 2017


labath added a comment.

In https://reviews.llvm.org/D34405#785627, @zturner wrote:

> In https://reviews.llvm.org/D34405#785598, @labath wrote:
>
> > I am not entirely thrilled by that, as that adds more bloat to the error  message ("llvm::detail::TakeExpected(std::move(original_assert_expression)) failed"), but I could go with that.
>
>
> I think we can fix that by using a full blown class based implementation of `MatcherImpl` instead of the `MATCHER_P` macro.  But it was a bit tricky so I didn't attempt it yet.


This idea intrigued me, so I tried playing around with it. Unfortunately, I have to report back that I think this is not possible. The first thing that the EXPECT_THAT macro does is pass the value through the `testing::internal::PredicateFormatterFromMatcher` class, which takes the value as `const T &`. From this point on, nothing we do can help as the value is already unmodifyable.

I was able to get this working by specializing this class and adding the overloads for other value categories, but this is a pretty big hack, and I think we don't want that. In light of that, I propose to stick to the current implementation. I think we should optimize for error message length, and having an extra TakeExpected overload is a small price to pay for that.


https://reviews.llvm.org/D34405





More information about the llvm-commits mailing list