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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
