ymandel marked 2 inline comments as done.
ymandel added inline comments.

================
Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:234
+DynTypedMatcher::constructWithTraversalKind(DynTypedMatcher InnerMatcher,
+                                            ast_type_traits::TraversalKind TK) 
{
+  InnerMatcher.Implementation =
----------------
gribozavr2 wrote:
> It might read better as an instance method on `DynTypedMatcher`: 
> `DynTypedMatcher::withTraversalKind()`. It is not unprecedented, see 
> `dynCastTo()`.
Agreed, thanks for the suggestion.


================
Comment at: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp:182
+  EXPECT_TRUE(TK.hasValue());
+  EXPECT_EQ(*TK, TK_AsIs);
+}
----------------
gribozavr2 wrote:
> Please use `EXPECT_THAT(M.getTraversalKind(), llvm::ValueIs(TK_AsIs));` (also 
> in tests below).
> 
> You'll need to include `llvm/Testing/Support/SupportHelpers.h`.
Thanks! This cleaned up the tests quite nicely.

I also updated the CMake file. Please let me know if that looks right to you. 
Cargo-culted from other cmake files...



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80685/new/

https://reviews.llvm.org/D80685



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to