[PATCH] D43330: [gtest] Add PrintTo overload for StringRef.

[Ilya Biryukov via Phabricator via llvm-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D43330#1008558, @labath wrote:

> This code does not have to live directly inside gtest. As such, maybe a better place for it would be somewhere under `include/llvm/Testing` ? (We already have `PrintTo` functions for `Error` and `Expected` there..)
>
> The reason I'm mentioning this is that while StringRef is a fairly basic class, I'm sure other classes would benefit from being GTEST-ified as well, and shoving them all somewhere into third party code sounds like a bad idea...


Thanks for mentioning those, I didn't know we had them.
Putting `PrintTo(StringRef)` into `llvm/Testing` would mean that clients will have to include it in order to get nice test output, otherwise they'll end up with the defaults and the code will compile, but the output for `StringRef` is currently totally unreadable, something like `{'#'(11), 'i' (12), 'n' (13), 'c' (14)}` (numbers represent the char codes).
I don't think the same is true for `Expected` and `Error`, they won't compile, so users will have to include the header in order to fix compile errors.

One alternative is putting the `PrintTo` function into `StringRef.h`, but that would mean we'll have to add unnecessary include to `StringRef.h` (`<ostream>`). So I would actually argue for keeping it in the gtest helpers. WDYT?


Repository:
  rL LLVM

https://reviews.llvm.org/D43330