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

[Zachary Turner via llvm-commits]

Oh I just meant improving the error message. Chandlerc has ideas on the
constness issue (probably involving const_cast)
On Wed, Jun 21, 2017 at 3:35 AM Pavel Labath via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170621/90bfbbb0/attachment.html>