[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions
Carlos Galvez via Phabricator via llvm-commits
llvm-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 llvm-commits
mailing list