[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

Carlos Galvez via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Oct 9 01:44:36 PDT 2021


carlosgalvezp added a comment.

Looking at the code, I see that you use `SuppressionIsSpecific` in order to separate the errors into 2 error lists: `SpecificNolintBegins` and `GlobalNolintBegins`. However you don't do anything different with either list, so why do they need to be different lists?

Here checking that a combined list is empty would be equivalent:

  bool WithinNolintBegin =
      !SpecificNolintBegins.empty() || !GlobalNolintBegins.empty();

And here, you are running identical code for both lists:

  for (const auto NolintBegin : SpecificNolintBegins) {
    auto Error = createNolintError(Context, SM, NolintBegin, true);
    SuppressionErrors.emplace_back(Error);
  }
  for (const auto NolintBegin : GlobalNolintBegins) {
    auto Error = createNolintError(Context, SM, NolintBegin, true);
    SuppressionErrors.emplace_back(Error);
  }

And then these lists are not used any further than the scope of the function where they are declared. So to me it feels like they could be combined, and this logic of `SuppressionIsSpecific` be removed. Let me know if I'm missing something obvious!


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

https://reviews.llvm.org/D111208



More information about the cfe-commits mailing list