[PATCH] D64863: [clangd] Ignore diags from builtin files

This revision was automatically updated to reflect the committed changes. Closed by commit rL367303: [clangd] Ignore diags from builtin files (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D64863: [clangd] Ignore diags from builtin files

kadircet updated this revision to Diff 212307. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64863/new/ https://reviews.llvm.org/D64863 Files: clang-tools-ex

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Many thanks for fixing this. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:111 -void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,

[PATCH] D64863: [clangd] Ignore diags from builtin files

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563 FillDiagBase(*LastDiag); -adjustDiagFromHeader(*LastDiag, Info, *LangOpts); +if (!InsideMainFile) + LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpts); -

[PATCH] D64863: [clangd] Ignore diags from builtin files

kadircet updated this revision to Diff 212131. kadircet marked 3 inline comments as done. kadircet added a comment. - Always set LastDiagWasAdjusted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64863/new/ https://reviews.llvm.org/D64863 Files:

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563 FillDiagBase(*LastDiag); -adjustDiagFromHeader(*LastDiag, Info, *LangOpts); +if (!InsideMainFile) + LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpt

[PATCH] D64863: [clangd] Ignore diags from builtin files

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563 FillDiagBase(*LastDiag); -adjustDiagFromHeader(*LastDiag, Info, *LangOpts); +if (!InsideMainFile) + LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpts); -

[PATCH] D64863: [clangd] Ignore diags from builtin files

kadircet updated this revision to Diff 210838. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64863/new/ https://reviews.llvm.org/D64863 Files: clang-tools-ex

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:128 +return false; + Position StartPos = sourceLocToPosition(SM, IncludeInMainFile); NIT: inline `StartPos`, it has online a single usage now. Comm

[PATCH] D64863: [clangd] Ignore diags from builtin files

kadircet updated this revision to Diff 210834. kadircet added a comment. - Move deduplication logic back into the `flushLastDiag` as discussed offline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64863/new/ https://reviews.llvm.org/D64863 Files:

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:601 - if (mentionsMainFile(*LastDiag) || - (LastDiag->Severity >= DiagnosticsEngine::Level::Error && - IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) { --

[PATCH] D64863: [clangd] Ignore diags from builtin files

kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:473 + SourceManager &SM = Info.getSourceManager(); + if (!InsideMainFile && SM.isWrittenInBuiltinFile(Info.getLocation())) { +IgnoreDiagnostics::l

[PATCH] D64863: [clangd] Ignore diags from builtin files

kadircet updated this revision to Diff 210786. kadircet added a comment. - Add tests. - Change the layer we ignore the diags: - Mark diags from headers as insidemainfile when we decide to surface them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:473 + SourceManager &SM = Info.getSourceManager(); + if (!InsideMainFile && SM.isWrittenInBuiltinFile(Info.getLocation())) { +IgnoreDiagnostics::log(DiagLevel, Info);

[PATCH] D64863: [clangd] Ignore diags from builtin files

hokein added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:473 + SourceManager &SM = Info.getSourceManager(); + if (!InsideMainFile && SM.isWrittenInBuiltinFile(Info.getLocation())) { +IgnoreDiagnostics::log(DiagLevel, Info); ilya-bi

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added a comment. Thanks for switching to `SM` everywhere, makes the code much more readable! A rough proposal for testing this: // test.cpp // RUN: clang++ -DFOO=b ./test.cpp int a = FOO; This produces a note inside a `` buffer. We could probably have something similar poin

[PATCH] D64863: [clangd] Ignore diags from builtin files

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay. Herald added a project: clang. This fixes a case where we show diagnostics on arbitrary lines, in an internal codebase. Open for ideas on unittesting this.