gribozavr added inline comments.
================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:472
if (Info.hasSourceManager())
checkFilters(Info.getLocation(), Info.getSourceManager());
}
----------------
nik wrote:
> gribozavr wrote:
> > It seems like the `checkFilters` call should not be skipped even if we have
> > another diagnostic engine.
> checkFilters() sets some state so that later finalizeLastError() can remove
> diagnostics/errors from ClangTidyDiagnosticConsumer::Errors and also track
> some statistics.
>
> Statistics are not relevant for the pluginc case as they should not be
> printed out for that case.
> The filtering happening in finalizeLastError() does not look relevant as the
> plugin currently only supports the "-checks=..." argument but not the e.g.
> header and line filter. When checks are created in
> ClangTidyCheckFactories::createChecks, the "-checks=..." argument is honored,
> so that the !Context.isCheckEnabled(...) in finalizeLastError() is also not
> relevant.
>
> I agree that checkFilters()/finalizeLastError() will be needed once the
> plugin case should support the other filtering options (header + line
> filter), but note that this goes beyond the scope of this change here, which
> is NOLINT filtering.
>
> This is all new to me, so if I miss something regarding the
> checkFilters()/finalizeLastError() thingy, please tell me, preferably with an
> example if possible :)
It might be not relevant right now, but that code has invariants about how
those two functions are called. Even if things happen to work, I think this
code is too complicated to allow breaking those invariants in some cases.
================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:207
CheckNamesByDiagnosticID.try_emplace(ID, CheckName);
+ CustomDiagIdParamsByDiagnosticID.try_emplace(
+ ID, CustomDiagIdParams(Level, FormatString));
----------------
Did you consider adding this query API to DiagnosticIDs instead? To me it looks
weird that creation of custom diag IDs is handled by one class (DiagnosticIDs),
and queries -- by another (ClangTidyContext).
================
Comment at: test/clang-tidy/nolintnextline-plugin.cpp:47
+
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin
-Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang
-checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
----------------
Why not on the first line?
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61487/new/
https://reviews.llvm.org/D61487
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
