[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood marked an inline comment as done.
LegalizeAdulthood added inline comments.


================
Comment at: clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210-214
+std::ostream &operator<<(std::ostream &Str,
+                         const clang::tidy::test::Param &Value) {
+  return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '"
+             << Value.Text << "'";
+}
----------------
LegalizeAdulthood wrote:
> aaronpuchert wrote:
> > LegalizeAdulthood wrote:
> > > LegalizeAdulthood wrote:
> > > > aaronpuchert wrote:
> > > > > Seems to have caused a [build failure](https://lab.llvm.org/buildbot/#/builders/57/builds/17915):
> > > > > 
> > > > > ```
> > > > > FAILED: tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o 
> > > > > /home/buildbots/clang.11.0.0/bin/clang++ --gcc-toolchain=/opt/rh/devtoolset-7/root/usr  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/unittests/clang-tidy -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include -Itools/clang/include -Iinclude -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-tidy -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googlemock/include -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG    -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o -MF tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o.d -o tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o -c /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
> > > > > /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210:15: error: unused function 'operator<<' [-Werror,-Wunused-function]
> > > > > std::ostream &operator<<(std::ostream &Str,
> > > > >               ^
> > > > > 1 error generated.
> > > > > ```
> > > > Simon Pilgrim fixed it, but I don't understand why clang calls this function unused.  When the test fails, gtest uses this function to pretty print the parameter.  I'm rebuilding with a forced test failure to validate.
> > > Yes, without this function the failing test prints results like this:
> > > ```
> > > [ RUN      ] TokenExpressionParserTests/MatcherTest.MatchResult/123
> > > D:\legalize\llvm\llvm-project\clang-tools-extra\unittests\clang-tidy\ModernizeModuleTest.cpp(200): error: Value of: matchText(GetParam().Text) == GetParam().Matched
> > >   Actual: false
> > > Expected: true
> > > [  FAILED  ] TokenExpressionParserTests/MatcherTest.MatchResult/123, where GetParam() = 16-byte object <00-00 00-00 00-00 00-00 40-EC 3B-D6 F6-7F 00-00> (1 ms)
> > > ```
> > > ....which isn't particularly useful.
> > > 
> > > So how do we include pretty printers for tests without clang erroneously flagging them as unused?
> > What got me wondering: this definition is last in the file, and there is no prior declaration of this function. How can there be any uses of it? We're not in class scope, so all prior uses of `operator<<` or perhaps `PrintTo` must have been resolved to some other function already. Or am I missing something?
> It seems what other tests do is define a friend function in the parameter class.  I'm going to push that and see if that is accepted.
Could have been an MSVC-ism, which is what I develop with.  It was most definitely being used while I was working on the test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124500