This is an archive of the discontinued LLVM Phabricator instance.

[llvm][Testing/ADT] Implement `IsStringMapEntry` testing matcher for verifying the entries in a `StringMap`.
ClosedPublic

Authored by wyt on Aug 26 2022, 12:22 PM.

Diff Detail

Event Timeline

wyt created this revision.Aug 26 2022, 12:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2022, 12:22 PM
Herald added a subscriber: mgorny. · View Herald Transcript
wyt requested review of this revision.Aug 26 2022, 12:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2022, 12:22 PM
gribozavr2 added inline comments.Aug 26 2022, 4:24 PM
llvm/include/llvm/ADT/StringMapEntry.h
103
llvm/include/llvm/Testing/ADT/StringMapEntryMatcher.h
20–21 ↗(On Diff #455993)
llvm/unittests/Testing/ADT/StringMapEntryMatcherTest.cpp
58 ↗(On Diff #455993)

Unused variable?

llvm/unittests/Testing/CMakeLists.txt
1–2 ↗(On Diff #455993)

Please sort.

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
4178

Could you put it right before TestingSupport (below), and call it TestingADT (to follow the existing camel case naming convention)?

wyt updated this revision to Diff 456314.Aug 29 2022, 5:14 AM
wyt marked 4 inline comments as done.

Address comments.

wyt added a reviewer: sgatev.Aug 30 2022, 2:27 AM

Did you accidentally revert the Bazel build files with the last update? LGTM otherwise.

wyt added a comment.Aug 30 2022, 6:05 AM

Did you accidentally revert the Bazel build files with the last update? LGTM otherwise.

The build files are now created in the parent patch (https://reviews.llvm.org/D132747) since the implementation for << has been moved into Testing/ADT.

wyt marked an inline comment as done.Aug 30 2022, 6:11 AM
wyt added inline comments.
utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
4178

Creation of the bazel target is moved to parent patch, and renaming has been updated there.

wyt marked an inline comment as done.Aug 30 2022, 6:12 AM
gribozavr2 accepted this revision.Aug 30 2022, 6:14 AM

Ah, I see, I didn't realize that other patch was a parent to this one.

This revision is now accepted and ready to land.Aug 30 2022, 6:14 AM
sgatev accepted this revision.Aug 30 2022, 8:52 AM
ymandel accepted this revision.Aug 30 2022, 9:00 AM
wyt edited the summary of this revision. (Show Details)Aug 30 2022, 9:44 AM
wyt updated this revision to Diff 457230.Sep 1 2022, 4:38 AM

Update build target.

This revision was landed with ongoing or failed builds.Sep 1 2022, 10:31 AM
This revision was automatically updated to reflect the committed changes.