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

[Alex Strelnikov via Phabricator via cfe-commits]

astrelni marked 4 inline comments as done.
astrelni added a comment.

In D62977#1560879 <https://reviews.llvm.org/D62977#1560879>, @lebedev.ri wrote:

> 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


The nosuite test was a strict subset. I've combined them with a few lines that needed to be turned via preprocessor branches. Thank you, I actually wasn't aware that these tests can have multiple `RUN` lines.



================
Comment at: clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h:34
+private:
+  std::unordered_set<unsigned> MatchedTemplateLocations;
+};
----------------
lebedev.ri wrote:
> Have you tried `llvm::DenseSet` instead?
> This //may// not matter *here*, but `std::unordered_set` usually results in horrible perf.
Thanks, I wasn't aware of what is available.


================
Comment at: clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case-nosuite.cpp:9
+void DummyFixTarget() {}
+// CHECK-FIXES: void DummyFixTarget() {}
+
----------------
lebedev.ri wrote:
> Hm?
I added a comment to better explain this in the combined test. check_clang_tidy.py asserts that there is at least one message, fix or note comment present in the file. If there isn't one, the script returns without even running the test. I couldn't see an option to pass that would turn of this check, please let me know if you are aware of one.


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

https://reviews.llvm.org/D62977