Page MenuHomePhabricator

[Support] Add a GTest matcher for Optional<T>
ClosedPublic

Authored by ilya-biryukov on Apr 24 2019, 8:36 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

ilya-biryukov created this revision.Apr 24 2019, 8:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2019, 8:36 AM
sammccall accepted this revision.Apr 24 2019, 10:10 AM
sammccall added inline comments.
llvm/include/llvm/Testing/Support/SupportHelpers.h
65 ↗(On Diff #196461)

AFAICS from other helpers, ValueIsMatcher should go in llvm::detail, and ValueIs should go in llvm.

unittest::getInputFileDirectory() reads well, but unittest::ValueIs(...) really doesn't...

85 ↗(On Diff #196461)

is this actually necessary? I think it's already going to print the value, which is always "None" - I'm not sure "None, which does not have a value" is clearer.

109 ↗(On Diff #196461)

it's a shame the name HasValue is taken, as I think it would much more clearly communicate "has a value *and* the value is...".

This revision is now accepted and ready to land.Apr 24 2019, 10:10 AM
ilya-biryukov marked 6 inline comments as done.
  • Move matcher class to llvm::detail, factory function to llvm
  • Add a simple test
ilya-biryukov added inline comments.Apr 25 2019, 1:57 AM
llvm/include/llvm/Testing/Support/SupportHelpers.h
65 ↗(On Diff #196461)

Agreed.

85 ↗(On Diff #196461)

Agree, that should be fine

109 ↗(On Diff #196461)

Totally agree, couldn't come up with a better name that wasn't taken.

Closed by commit rL359174: [Support] Add a GTest matcher for Optional<T> (authored by ibiryukov, committed by ). · Explain WhyApr 25 2019, 2:01 AM
This revision was automatically updated to reflect the committed changes.