[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Florin Iucha via Phabricator via cfe-commits]

0x8000-0000 added a comment.

Summary of "misses"

One:

  uint32_t counter = 0;
  BOOST_FOREACH(const Foo* foo, *theFoos)
  {
      if (foo->getType() == someType)
      {
          ++counter;
      }
  }

The -fix makes the counter const :), so obviously it breaks compilation.

Two:

It marks a lot of std::stringstream / std::ostringstream instances as const. So we can't << into it, breaking compilation.

Three:

It marked an array in the header as const. There were some pointer variables (in a translation unit which included that header) that were declared and initialized to 0 (meaning nullptr), then later in a case block of a switch they were assigned &array[0] value. It did not change the pointer variables, but they used to be mutable pointer to mutable, so now the assignment from a const array failed to build.

The changes required to get the build complete (7500 translation units);

  15 files changed, 59 insertions(+), 59 deletions(-)

I can live with the breakage in the 3rd case - since the fix is one time and straightforward (and makes the code clearly better). But I prefer not to have to annotate all std::sotringstream instances with NOLINT. Also we have a fair number of BOOST_FOREACH macros from way back then. They should be ripped out of course, but it's in code that wasn't touched in a while, so having the clang-tidy keep reporting / misfixing that area is bothersome.

I really want this clang-tidy tool, but I don't want to put off users via false positives. Many don't want to deal with it anyway, and the smallest excuse will be brought up against enabling the use of the tool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943