Commit fcfd891f authored by Hans Wennborg's avatar Hans Wennborg
Browse files

Merging r367303: 

------------------------------------------------------------------------
r367303 | kadircet | 2019-07-30 12:26:51 +0200 (Tue, 30 Jul 2019) | 15 lines

[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
------------------------------------------------------------------------

llvm-svn: 368682
parent ff20769b

clang-tools-extra/clangd/Diagnostics.cpp

+22 −14
Original line number Diff line number Diff line
@@ -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::Diagnostic &D, const LangOptions &L) { 
  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 clang::Diagnostic &Info, 
       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 clang::Diagnostic &Info, 


  // 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(DiagnosticsEngine::Level DiagLevel, 
  }



  bool InsideMainFile = isInsideMainFile(Info);

  SourceManager &SM = Info.getSourceManager();



  auto FillDiagBase = [&](DiagBase &D) {

    D.Range = diagnosticRange(Info, *LangOpts);
@@ -472,8 +481,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, 
    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(DiagnosticsEngine::Level DiagLevel, 
      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(DiagnosticsEngine::Level DiagLevel, 
    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(DiagnosticsEngine::Level DiagLevel, 
    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(DiagnosticsEngine::Level DiagLevel, 
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 {

clang-tools-extra/clangd/Diagnostics.h

+2 −0
Original line number Diff line number Diff line
@@ -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;

};

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

+13 −1
Original line number Diff line number Diff line
@@ -928,7 +928,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 "
@@ -936,6 +935,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