[clang] ec66603 - [clang-format] Remove the dependency on frontend

Vlad Tsyrklevich via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 29 10:51:37 PDT 2019


I've reverted this commit as it was causing UBSan failures on the ubsan
bot. These failures looked like:
llvm/lib/Support/SourceMgr.cpp:440:48: runtime error: pointer index
expression with base 0x000000000000 overflowed to 0xfffffffffffffffa

Looking at a backtrace, this line was reached from the
`Diags.print(nullptr, llvm::errs(), (ShowColors && !NoShowColors));` call
introduced in this change.

I apologize for the delay in this revert, normally reverts happen in under
a day but in this case this change was committed right as we switched to
GitHub and it took a while to get the sanitizers buildbots working properly
and track down failures.

On Thu, Oct 24, 2019 at 11:17 AM via cfe-commits <cfe-commits at lists.llvm.org>
wrote:

>
> Author: paulhoad
> Date: 2019-10-24T19:03:57+01:00
> New Revision: ec66603ac7ea655be5c2c5f508c5bf0d5eaeb65b
>
> URL:
> https://github.com/llvm/llvm-project/commit/ec66603ac7ea655be5c2c5f508c5bf0d5eaeb65b
> DIFF:
> https://github.com/llvm/llvm-project/commit/ec66603ac7ea655be5c2c5f508c5bf0d5eaeb65b.diff
>
> LOG: [clang-format] Remove the dependency on frontend
>
> Summary:
> Address review comments from {D68554} by trying to drop the dependency
> again on Frontend whilst keeping the same format diagnostic messages
>
> Not completely happy with having to do a split in order to get the
> StringRef for the Line the error occurred on, but could see a way to use
> SourceManager and SourceLocation to give me a single line?
>
> But this removes the dependency on frontend which should keep the binary
> size down.
>
> Reviewers: thakis, klimek, mitchell-stellar
>
> Reviewed By: klimek
>
> Subscribers: mgorny, cfe-commits
>
> Tags: #clang, #clang-format
>
> Differential Revision: https://reviews.llvm.org/D68969
>
> Added:
>
>
> Modified:
>     clang/tools/clang-format/CMakeLists.txt
>     clang/tools/clang-format/ClangFormat.cpp
>
> Removed:
>
>
>
>
> ################################################################################
> diff  --git a/clang/tools/clang-format/CMakeLists.txt
> b/clang/tools/clang-format/CMakeLists.txt
> index 28ac4fb5913e..35ecdb11253c 100644
> --- a/clang/tools/clang-format/CMakeLists.txt
> +++ b/clang/tools/clang-format/CMakeLists.txt
> @@ -7,7 +7,6 @@ add_clang_tool(clang-format
>  set(CLANG_FORMAT_LIB_DEPS
>    clangBasic
>    clangFormat
> -  clangFrontend
>    clangRewrite
>    clangToolingCore
>    )
>
> diff  --git a/clang/tools/clang-format/ClangFormat.cpp
> b/clang/tools/clang-format/ClangFormat.cpp
> index f39c18bae3ff..a10541d88f07 100644
> --- a/clang/tools/clang-format/ClangFormat.cpp
> +++ b/clang/tools/clang-format/ClangFormat.cpp
> @@ -18,7 +18,6 @@
>  #include "clang/Basic/SourceManager.h"
>  #include "clang/Basic/Version.h"
>  #include "clang/Format/Format.h"
> -#include "clang/Frontend/TextDiagnosticPrinter.h"
>  #include "clang/Rewrite/Core/Rewriter.h"
>  #include "llvm/Support/CommandLine.h"
>  #include "llvm/Support/FileSystem.h"
> @@ -325,12 +324,9 @@ emitReplacementWarnings(const Replacements &Replaces,
> StringRef AssumedFileName,
>    IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new
> DiagnosticOptions();
>    DiagOpts->ShowColors = (ShowColors && !NoShowColors);
>
> -  TextDiagnosticPrinter *DiagsBuffer =
> -      new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
> -
>    IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
>    IntrusiveRefCntPtr<DiagnosticsEngine> Diags(
> -      new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
> +      new DiagnosticsEngine(DiagID, &*DiagOpts));
>
>    IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
>        new llvm::vfs::InMemoryFileSystem);
> @@ -339,24 +335,40 @@ emitReplacementWarnings(const Replacements
> &Replaces, StringRef AssumedFileName,
>    FileID FileID = createInMemoryFile(AssumedFileName, Code.get(), Sources,
>                                       Files, InMemoryFileSystem.get());
>
> -  const unsigned ID = Diags->getCustomDiagID(
> -      WarningsAsErrors ? clang::DiagnosticsEngine::Error
> -                       : clang::DiagnosticsEngine::Warning,
> -      "code should be clang-formatted [-Wclang-format-violations]");
> +  FileManager &FileMgr = Sources.getFileManager();
> +  llvm::ErrorOr<const FileEntry *> FileEntryPtr =
> +      FileMgr.getFile(AssumedFileName);
>
>    unsigned Errors = 0;
> -  DiagsBuffer->BeginSourceFile(LangOptions(), nullptr);
>    if (WarnFormat && !NoWarnFormat) {
>      for (const auto &R : Replaces) {
> -      Diags->Report(
> -
> Sources.getLocForStartOfFile(FileID).getLocWithOffset(R.getOffset()),
> -          ID);
> +      PresumedLoc PLoc = Sources.getPresumedLoc(
> +
> Sources.getLocForStartOfFile(FileID).getLocWithOffset(R.getOffset()));
> +
> +      SourceLocation LineBegin =
> +          Sources.translateFileLineCol(FileEntryPtr.get(),
> PLoc.getLine(), 1);
> +      SourceLocation NextLineBegin = Sources.translateFileLineCol(
> +          FileEntryPtr.get(), PLoc.getLine() + 1, 1);
> +
> +      const char *StartBuf = Sources.getCharacterData(LineBegin);
> +      const char *EndBuf = Sources.getCharacterData(NextLineBegin);
> +
> +      StringRef Line(StartBuf, (EndBuf - StartBuf) - 1);
> +
> +      SMDiagnostic Diags(
> +          llvm::SourceMgr(), SMLoc(), AssumedFileName, PLoc.getLine(),
> +          PLoc.getColumn(),
> +          WarningsAsErrors ? SourceMgr::DiagKind::DK_Error
> +                           : SourceMgr::DiagKind::DK_Warning,
> +          "code should be clang-formatted [-Wclang-format-violations]",
> Line,
> +          ArrayRef<std::pair<unsigned, unsigned>>());
> +
> +      Diags.print(nullptr, llvm::errs(), (ShowColors && !NoShowColors));
>        Errors++;
>        if (ErrorLimit && Errors >= ErrorLimit)
>          break;
>      }
>    }
> -  DiagsBuffer->EndSourceFile();
>    return WarningsAsErrors;
>  }
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191029/0455ae33/attachment.html>


More information about the cfe-commits mailing list