Skip to content

Add the IsA matcher for matching PolymorphicBase*.#4427

Merged
wujingyue merged 3 commits intomainfrom
wjy/match
May 13, 2025
Merged

Add the IsA matcher for matching PolymorphicBase*.#4427
wujingyue merged 3 commits intomainfrom
wjy/match

Conversation

@wujingyue
Copy link
Collaborator

No description provided.

@wujingyue
Copy link
Collaborator Author

!test

@wujingyue wujingyue requested review from nsarka and zasdfgbnm May 12, 2025 18:23
@github-actions
Copy link

github-actions bot commented May 12, 2025

Review updated until commit 1198265

Description

  • Add IsA matcher for polymorphic base matching.

  • Replace Property and IsTrue with IsA in tests.


Changes walkthrough 📝

Relevant files
Enhancement
test_expr_sort.cpp
Replace Property and IsTrue with IsA                                         

tests/cpp/test_expr_sort.cpp

  • Removed unused gmock-more-matchers.h include.
  • Replaced Property and IsTrue with IsA in TEST_F(ExprSortTest,
    SegmentedGroup_Binary_SameOperand).
  • +1/-6     
    test_host_ir_integration.cpp
    Replace Property and IsTrue with IsA                                         

    tests/cpp/test_host_ir_integration.cpp

  • Removed unused gmock-more-matchers.h include.
  • Replaced Property and IsTrue with IsA in TEST_F(HostIrIntegrationTest,
    InsertDeallocations).
  • +1/-7     
    test_move_pad.cpp
    Replace Property and IsTrue with IsA                                         

    tests/cpp/test_move_pad.cpp

  • Removed unused gmock-more-matchers.h include.
  • Replaced Property and IsTrue with IsA in TEST_F(MovePadTest,
    CascadePadCase0) and TEST_F(MovePadTest, CascadePadCase1).
  • +2/-5     
    test_move_split_cat.cpp
    Replace Property and IsTrue with IsA                                         

    tests/cpp/test_move_split_cat.cpp

  • Removed unused gmock-more-matchers.h include.
  • Replaced Property and IsTrue with IsA in TEST_F(MoveSplitCatTest,
    MultiplePairs) and TEST_F(MoveSplitCatTest, MultipleCatsOnSameSplit).
  • +3/-7     
    test_multidevice_lower_communication.cpp
    Replace Property and IsTrue with IsA                                         

    tests/cpp/test_multidevice_lower_communication.cpp

  • Removed unused gmock-more-matchers.h include.
  • Replaced Property and IsTrue with IsA in
    assertIsCompiledToHostIrContainer.
  • +2/-13   
    validator.h
    Add IsA matcher for polymorphic base matching                       

    tests/cpp/validator.h

  • Added gmock-more-matchers.h include.
  • Added IsA matcher template for polymorphic base matching.
  • +30/-0   

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🧪 PR contains tests
    ⚡ Recommended focus areas for review

    Possible Issue

    The new IsA matcher is used without any tests or examples of its usage in the PR description or comments. Ensure that the new matcher works as expected and add relevant tests if necessary.

    using testing::ElementsAre;
    using testing::SizeIs;
    
    Documentation

    The new IsA matcher is introduced but lacks detailed documentation. Consider adding comments or documentation to explain how to use the matcher and its intended purpose.

    class IsAMatcher {
     public:
      using is_gtest_matcher = void;
    
      bool MatchAndExplain(const PolymorphicBase* pb, std::ostream*) const {
        return pb->isA<T>();
      }
    
      void DescribeTo(std::ostream* os) const {
        *os << "is " << demangle(typeid(T).name());
      }
    
      void DescribeNegationTo(std::ostream* os) const {
        *os << "is not " << demangle(typeid(T).name());
      }
    };
    
    template <typename T>
    inline testing::Matcher<const PolymorphicBase*> IsA() {
      return IsAMatcher<T>();
    }

    @wujingyue
    Copy link
    Collaborator Author

    !test

    @wujingyue
    Copy link
    Collaborator Author

    !test

    @wujingyue wujingyue merged commit d0e2a80 into main May 13, 2025
    41 of 43 checks passed
    @wujingyue wujingyue deleted the wjy/match branch May 13, 2025 18:11
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    None yet

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants