Merged to release_90 in r368682.
On Tue, Jul 30, 2019 at 12:26 PM Kadir Cetinkaya via cfe-commits <cfe-commits@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@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
