jansvoboda11 added inline comments.

================
Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:267
 
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fdebug-pass-manager")));
+  ASSERT_EQ(count(GeneratedArgs, "-fdebug-pass-manager"), 1);
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-debug-pass-manager"))));
----------------
dexonsmith wrote:
> jansvoboda11 wrote:
> > Is it wise to rely on pointer comparison here? The call to `count` returns 
> > 2 before changing the denormalizer and 1 after, but I'm not sure if it will 
> > work on all platforms.
> Does this compile / avoid the pointer comparison?
> ```
> ASSERT_EQ(count(GeneratedArgs, StringRef("-fdebug-pass-manager")), 1);
> ```
Yes, this forces the `const char *` from GeneratedArgs to be converted into 
`StringRef`, which does comparison via `::memcmp`.

If we decide comparing `StringRef`s is a better solution, I'd be inclined to 
create a helper function or custom matcher that does the conversion to 
`StringRef`. That way, we don't have to worry about doing the right thing in 
test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93094

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

Reply via email to