[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 27 07:50:37 PDT 2019


lebedev.ri resigned from this revision.
lebedev.ri added a comment.

Looks reasonable. I did not review the check itself though.
Are `test/clang-tidy/google-upgrade-googletest-case-nosuite.cpp` and `test/clang-tidy/google-upgrade-googletest-case.cpp ` identical other than the included header and expected output?
I'd recommend to condense it into a single file, and just have two `RUN` lines each one checking different message prefixes



================
Comment at: clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h:34
+private:
+  std::unordered_set<unsigned> MatchedTemplateLocations;
+};
----------------
Have you tried `llvm::DenseSet` instead?
This //may// not matter *here*, but `std::unordered_set` usually results in horrible perf.


================
Comment at: clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case-nosuite.cpp:9
+void DummyFixTarget() {}
+// CHECK-FIXES: void DummyFixTarget() {}
+
----------------
Hm?


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

https://reviews.llvm.org/D62977





More information about the cfe-commits mailing list