[clang-tools-extra] r367303 - [clangd] Ignore diags from builtin files
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 13 05:37:07 PDT 2019
Merged to release_90 in r368682.
On Tue, Jul 30, 2019 at 12:26 PM Kadir Cetinkaya via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> Author: kadircet
> Date: Tue Jul 30 03:26:51 2019
> New Revision: 367303
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367303&view=rev
> Log:
> [clangd] Ignore diags from builtin files
>
> Summary:
> This fixes a case where we show diagnostics on arbitrary lines, in an
> internal codebase.
>
> Open for ideas on unittesting this.
>
> Reviewers: ilya-biryukov
>
> Subscribers: MaskRay, jkorous, arphaman, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D64863
>
> Modified:
> clang-tools-extra/trunk/clangd/Diagnostics.cpp
> clang-tools-extra/trunk/clangd/Diagnostics.h
> clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=367303&r1=367302&r2=367303&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
> +++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Tue Jul 30 03:26:51 2019
> @@ -20,6 +20,7 @@
> #include "clang/Lex/Lexer.h"
> #include "clang/Lex/Token.h"
> #include "llvm/ADT/ArrayRef.h"
> +#include "llvm/ADT/DenseSet.h"
> #include "llvm/ADT/StringRef.h"
> #include "llvm/ADT/Twine.h"
> #include "llvm/Support/Capacity.h"
> @@ -107,8 +108,13 @@ Range diagnosticRange(const clang::Diagn
> return halfOpenToRange(M, R);
> }
>
> -void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
> +// Returns whether the \p D is modified.
> +bool adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
> const LangOptions &LangOpts) {
> + // We only report diagnostics with at least error severity from headers.
> + if (D.Severity < DiagnosticsEngine::Level::Error)
> + return false;
> +
> const SourceLocation &DiagLoc = Info.getLocation();
> const SourceManager &SM = Info.getSourceManager();
> SourceLocation IncludeInMainFile;
> @@ -119,13 +125,14 @@ void adjustDiagFromHeader(Diag &D, const
> IncludeLocation = GetIncludeLoc(IncludeLocation))
> IncludeInMainFile = IncludeLocation;
> if (IncludeInMainFile.isInvalid())
> - return;
> + return false;
>
> // Update diag to point at include inside main file.
> D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
> D.Range.start = sourceLocToPosition(SM, IncludeInMainFile);
> D.Range.end = sourceLocToPosition(
> SM, Lexer::getLocForEndOfToken(IncludeInMainFile, 0, SM, LangOpts));
> + D.InsideMainFile = true;
>
> // Add a note that will point to real diagnostic.
> const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
> @@ -138,6 +145,7 @@ void adjustDiagFromHeader(Diag &D, const
>
> // Update message to mention original file.
> D.Message = llvm::Twine("in included file: ", D.Message).str();
> + return true;
> }
>
> bool isInsideMainFile(const clang::Diagnostic &D) {
> @@ -465,6 +473,7 @@ void StoreDiags::HandleDiagnostic(Diagno
> }
>
> bool InsideMainFile = isInsideMainFile(Info);
> + SourceManager &SM = Info.getSourceManager();
>
> auto FillDiagBase = [&](DiagBase &D) {
> D.Range = diagnosticRange(Info, *LangOpts);
> @@ -472,8 +481,7 @@ void StoreDiags::HandleDiagnostic(Diagno
> Info.FormatDiagnostic(Message);
> D.Message = Message.str();
> D.InsideMainFile = InsideMainFile;
> - D.File = Info.getSourceManager().getFilename(Info.getLocation());
> - auto &SM = Info.getSourceManager();
> + D.File = SM.getFilename(Info.getLocation());
> D.AbsFile = getCanonicalPath(
> SM.getFileEntryForID(SM.getFileID(Info.getLocation())), SM);
> D.Severity = DiagLevel;
> @@ -496,10 +504,9 @@ void StoreDiags::HandleDiagnostic(Diagno
> if (FixIt.RemoveRange.getBegin().isMacroID() ||
> FixIt.RemoveRange.getEnd().isMacroID())
> return false;
> - if (!isInsideMainFile(FixIt.RemoveRange.getBegin(),
> - Info.getSourceManager()))
> + if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM))
> return false;
> - Edits.push_back(toTextEdit(FixIt, Info.getSourceManager(), *LangOpts));
> + Edits.push_back(toTextEdit(FixIt, SM, *LangOpts));
> }
>
> llvm::SmallString<64> Message;
> @@ -507,8 +514,8 @@ void StoreDiags::HandleDiagnostic(Diagno
> if (SyntheticMessage && Info.getNumFixItHints() == 1) {
> const auto &FixIt = Info.getFixItHint(0);
> bool Invalid = false;
> - llvm::StringRef Remove = Lexer::getSourceText(
> - FixIt.RemoveRange, Info.getSourceManager(), *LangOpts, &Invalid);
> + llvm::StringRef Remove =
> + Lexer::getSourceText(FixIt.RemoveRange, SM, *LangOpts, &Invalid);
> llvm::StringRef Insert = FixIt.CodeToInsert;
> if (!Invalid) {
> llvm::raw_svector_ostream M(Message);
> @@ -553,7 +560,9 @@ void StoreDiags::HandleDiagnostic(Diagno
> LastDiag = Diag();
> LastDiag->ID = Info.getID();
> FillDiagBase(*LastDiag);
> - adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
> + LastDiagWasAdjusted = false;
> + if (!InsideMainFile)
> + LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
>
> if (!Info.getFixItHints().empty())
> AddFix(true /* try to invent a message instead of repeating the diag */);
> @@ -595,10 +604,9 @@ void StoreDiags::HandleDiagnostic(Diagno
> void StoreDiags::flushLastDiag() {
> if (!LastDiag)
> return;
> - // Only keeps diagnostics inside main file or the first one coming from a
> - // header.
> - if (mentionsMainFile(*LastDiag) ||
> - (LastDiag->Severity >= DiagnosticsEngine::Level::Error &&
> + if (mentionsMainFile(*LastDiag) &&
> + (!LastDiagWasAdjusted ||
> + // Only report the first diagnostic coming from each particular header.
> IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) {
> Output.push_back(std::move(*LastDiag));
> } else {
>
> Modified: clang-tools-extra/trunk/clangd/Diagnostics.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.h?rev=367303&r1=367302&r2=367303&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/Diagnostics.h (original)
> +++ clang-tools-extra/trunk/clangd/Diagnostics.h Tue Jul 30 03:26:51 2019
> @@ -145,6 +145,8 @@ private:
> std::vector<Diag> Output;
> llvm::Optional<LangOptions> LangOpts;
> llvm::Optional<Diag> LastDiag;
> + /// Set iff adjustDiagFromHeader resulted in changes to LastDiag.
> + bool LastDiagWasAdjusted = false;
> llvm::DenseSet<int> IncludeLinesWithErrors;
> bool LastPrimaryDiagnosticWasSuppressed = false;
> };
>
> Modified: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp?rev=367303&r1=367302&r2=367303&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp (original)
> +++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp Tue Jul 30 03:26:51 2019
> @@ -907,7 +907,6 @@ TEST(DiagsInHeaders, OnlyErrorOrFatal) {
> int x = 5/0;)cpp");
> TestTU TU = TestTU::withCode(Main.code());
> TU.AdditionalFiles = {{"a.h", Header.code()}};
> - auto diags = TU.build().getDiagnostics();
> EXPECT_THAT(TU.build().getDiagnostics(),
> UnorderedElementsAre(AllOf(
> Diag(Main.range(), "in included file: C++ requires "
> @@ -915,6 +914,19 @@ TEST(DiagsInHeaders, OnlyErrorOrFatal) {
> WithNote(Diag(Header.range(), "error occurred here")))));
> }
>
> +TEST(IgnoreDiags, FromNonWrittenSources) {
> + Annotations Main(R"cpp(
> + #include [["a.h"]]
> + void foo() {})cpp");
> + Annotations Header(R"cpp(
> + int x = 5/0;
> + int b = [[FOO]];)cpp");
> + TestTU TU = TestTU::withCode(Main.code());
> + TU.AdditionalFiles = {{"a.h", Header.code()}};
> + TU.ExtraArgs = {"-DFOO=NOOO"};
> + EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
> +}
> +
> } // namespace
>
> } // namespace clangd
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list