[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-11-05 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

see D69854: [clang-format] [RELAND] Remove the dependency on frontend 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-11-05 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

In D68969#1733946 , @thakis wrote:

> The revert message said:
>
>   Vlad Tsyrklevich via cfe-commits 
>   Tue, Oct 29, 1:51 PM (7 days ago)
>   to via, mydeveloperday
>  
>   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 0x overflowed to 0xfffa
>  
>   Looking at a backtrace, this line was reached from the 
> `Diags.print(nullptr, llvm::errs(), (ShowColors && !NoShowColors));` call 
> introduced in this change.
>
>
> Is that enough to see what's wrong? If not, @vlad.tsyrklevich , can you give 
> repro steps for that ubsan failure that made you revert ec66603 in efed314 
> ?


Note to self: Took me a bit to set up a docker container to build with clang-10 
on linux (I'm normally a windows guy), but hopefully, I can debug from here...

  test1.cpp:1:7: warning: code should be clang-formatted 
[-Wclang-format-violations]
  /llvm-project/llvm/lib/Support/SourceMgr.cpp:440:48: runtime error: applying 
non-zero offset 1844674407
  3709551610 to null pointer
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior 
/buildareas/llvm2/llvm-project/llvm/lib/Support/SourceMgr.cpp:44
  0:48 in


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-11-05 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a subscriber: vlad.tsyrklevich.
thakis added a comment.

The revert message said:

  Vlad Tsyrklevich via cfe-commits 
  Tue, Oct 29, 1:51 PM (7 days ago)
  to via, mydeveloperday
  
  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 0x overflowed to 0xfffa
  
  Looking at a backtrace, this line was reached from the `Diags.print(nullptr, 
llvm::errs(), (ShowColors && !NoShowColors));` call introduced in this change.

Is that enough to see what's wrong? If not, @vlad.tsyrklevich , can you give 
repro steps for that ubsan failure that made you revert ec66603 in efed314 
?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

In D68969#1732489 , @thakis wrote:

> Since this was reverted: are you looking into relanding this?


I'm not totally sure how they reproduced the UBSAN issue


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-11-04 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Since this was reverted: are you looking into relanding this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-24 Thread MyDeveloperDay via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
MyDeveloperDay marked an inline comment as done.
Closed by commit rGec66603ac7ea: [clang-format] Remove the dependency on 
frontend (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969

Files:
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-format/ClangFormat.cpp


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ 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 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowColors = (ShowColors && !NoShowColors);
 
-  TextDiagnosticPrinter *DiagsBuffer =
-  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
-
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
   IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
+  new DiagnosticsEngine(DiagID, &*DiagOpts));
 
   IntrusiveRefCntPtr InMemoryFileSystem(
   new llvm::vfs::InMemoryFileSystem);
@@ -339,24 +335,40 @@
   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  = Sources.getFileManager();
+  llvm::ErrorOr FileEntryPtr =
+  FileMgr.getFile(AssumedFileName);
 
   unsigned Errors = 0;
-  DiagsBuffer->BeginSourceFile(LangOptions(), nullptr);
   if (WarnFormat && !NoWarnFormat) {
 for (const auto  : 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>());
+
+  Diags.print(nullptr, llvm::errs(), (ShowColors && !NoShowColors));
   Errors++;
   if (ErrorLimit && Errors >= ErrorLimit)
 break;
 }
   }
-  DiagsBuffer->EndSourceFile();
   return WarningsAsErrors;
 }
 
Index: clang/tools/clang-format/CMakeLists.txt
===
--- clang/tools/clang-format/CMakeLists.txt
+++ clang/tools/clang-format/CMakeLists.txt
@@ -7,7 +7,6 @@
 set(CLANG_FORMAT_LIB_DEPS
   clangBasic
   clangFormat
-  clangFrontend
   clangRewrite
   clangToolingCore
   )


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ 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 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowColors = (ShowColors && !NoShowColors);
 
-  TextDiagnosticPrinter *DiagsBuffer =
-  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
-
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
   IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
+  new DiagnosticsEngine(DiagID, &*DiagOpts));
 
   IntrusiveRefCntPtr InMemoryFileSystem(
   new llvm::vfs::InMemoryFileSystem);
@@ -339,24 +335,40 @@
   FileID FileID = createInMemoryFile(AssumedFileName, Code.get(), Sources,
  Files, InMemoryFileSystem.get());

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-22 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/tools/clang-format/ClangFormat.cpp:356
+
+  StringRef Line(StartBuf, (EndBuf - StartBuf) - 1);
+

klimek wrote:
> - 1 is to exclude the \n I'd assume?
correct, without the -1 I end up with an extra blank line in the output


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-21 Thread Manuel Klimek via Phabricator via cfe-commits 
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg




Comment at: clang/tools/clang-format/ClangFormat.cpp:356
+
+  StringRef Line(StartBuf, (EndBuf - StartBuf) - 1);
+

- 1 is to exclude the \n I'd assume?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-21 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

klimek: ping :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-17 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay updated this revision to Diff 225455.
MyDeveloperDay added a comment.

Update out by one errors and unused variables


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969

Files:
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-format/ClangFormat.cpp


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ 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 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowColors = (ShowColors && !NoShowColors);
 
-  TextDiagnosticPrinter *DiagsBuffer =
-  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
-
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
   IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
+  new DiagnosticsEngine(DiagID, &*DiagOpts));
 
   IntrusiveRefCntPtr InMemoryFileSystem(
   new llvm::vfs::InMemoryFileSystem);
@@ -339,24 +335,40 @@
   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  = Sources.getFileManager();
+  llvm::ErrorOr FileEntryPtr =
+  FileMgr.getFile(AssumedFileName);
 
   unsigned Errors = 0;
-  DiagsBuffer->BeginSourceFile(LangOptions(), nullptr);
   if (WarnFormat && !NoWarnFormat) {
 for (const auto  : 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>());
+
+  Diags.print(nullptr, llvm::errs(), (ShowColors && !NoShowColors));
   Errors++;
   if (ErrorLimit && Errors >= ErrorLimit)
 break;
 }
   }
-  DiagsBuffer->EndSourceFile();
   return WarningsAsErrors;
 }
 
Index: clang/tools/clang-format/CMakeLists.txt
===
--- clang/tools/clang-format/CMakeLists.txt
+++ clang/tools/clang-format/CMakeLists.txt
@@ -7,7 +7,6 @@
 set(CLANG_FORMAT_LIB_DEPS
   clangBasic
   clangFormat
-  clangFrontend
   clangRewrite
   clangToolingCore
   )


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ 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 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowColors = (ShowColors && !NoShowColors);
 
-  TextDiagnosticPrinter *DiagsBuffer =
-  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
-
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
   IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
+  new DiagnosticsEngine(DiagID, &*DiagOpts));
 
   IntrusiveRefCntPtr InMemoryFileSystem(
   new llvm::vfs::InMemoryFileSystem);
@@ -339,24 +335,40 @@
   FileID FileID = createInMemoryFile(AssumedFileName, Code.get(), Sources,
  Files, InMemoryFileSystem.get());
 
-  const unsigned ID = Diags->getCustomDiagID(
-  WarningsAsErrors ? clang::DiagnosticsEngine::Error
-   :

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-17 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added inline comments.



Comment at: clang/tools/clang-format/ClangFormat.cpp:351
+  SourceLocation LineBegin = Sources.translateFileLineCol(
+  FileEntryPtr.get(), PLoc.getLine() - 1, 0);
+  SourceLocation NextLineBegin =

MyDeveloperDay wrote:
> klimek wrote:
> > This is unexpected: I'd have expected this to be PLoc.getLine() here and 
> > PLoc.getLine() + 1 below. I'm probably missing something?
> This caught me out too, but it kept pulling the next line. So I think it must 
> be a zero based line as opposed to PesumedLoc which uses line 1 to mean the 
> first line..
> 
> but leet me double check.
OK this then perhaps is wrong, given the assert.. let me double check what is 
going on.

```
 SourceLocation SourceManager::translateFileLineCol(const FileEntry *SourceFile,
   unsigned Line,
   unsigned Col) const {
   assert(SourceFile && "Null source file!");
   assert(Line && Col && "Line and column should start from 1!");
 
   FileID FirstFID = translateFile(SourceFile);
   return translateLineCol(FirstFID, Line, Col);
 }
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-17 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added a comment.






Comment at: clang/tools/clang-format/ClangFormat.cpp:345
   if (WarnFormat && !NoWarnFormat) {
+ArrayRef> Ranges;
 for (const auto  : Replaces) {

klimek wrote:
> Looks unused?
correct, I changed my mind and now call ArrayRef>() as the last argument not sure if thay would be slower than me 
creating one empty one outside the loop, any thoughts? I'll go with a consensus 
and will remove the other.



Comment at: clang/tools/clang-format/ClangFormat.cpp:351
+  SourceLocation LineBegin = Sources.translateFileLineCol(
+  FileEntryPtr.get(), PLoc.getLine() - 1, 0);
+  SourceLocation NextLineBegin =

klimek wrote:
> This is unexpected: I'd have expected this to be PLoc.getLine() here and 
> PLoc.getLine() + 1 below. I'm probably missing something?
This caught me out too, but it kept pulling the next line. So I think it must 
be a zero based line as opposed to PesumedLoc which uses line 1 to mean the 
first line..

but leet me double check.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-17 Thread Manuel Klimek via Phabricator via cfe-commits 
klimek added inline comments.



Comment at: clang/tools/clang-format/ClangFormat.cpp:345
   if (WarnFormat && !NoWarnFormat) {
+ArrayRef> Ranges;
 for (const auto  : Replaces) {

Looks unused?



Comment at: clang/tools/clang-format/ClangFormat.cpp:351
+  SourceLocation LineBegin = Sources.translateFileLineCol(
+  FileEntryPtr.get(), PLoc.getLine() - 1, 0);
+  SourceLocation NextLineBegin =

This is unexpected: I'd have expected this to be PLoc.getLine() here and 
PLoc.getLine() + 1 below. I'm probably missing something?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-16 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay updated this revision to Diff 225320.
MyDeveloperDay added a comment.

Remove need to split lines or search for `\n`

General Algorithm

1. take Location of Replacement line and column
2. make a new SourceLocation of line and column =0 (beginning of that 
replacement line)
3. make a new SourceLocation of line+1 and column = 0   (beginning of the next 
line)
4. use `getCharacterData()` to get pointers to beginning of each line
5. make a String from those begin and end ptrs

Seems very much faster when run over clang source tree (removal of noticeable 
pauses on large files)

That should equal the line


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969

Files:
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-format/ClangFormat.cpp


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -18,8 +18,8 @@
 #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/Demangle/StringView.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/InitLLVM.h"
@@ -325,12 +325,9 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowColors = (ShowColors && !NoShowColors);
 
-  TextDiagnosticPrinter *DiagsBuffer =
-  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
-
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
   IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
+  new DiagnosticsEngine(DiagID, &*DiagOpts));
 
   IntrusiveRefCntPtr InMemoryFileSystem(
   new llvm::vfs::InMemoryFileSystem);
@@ -339,24 +336,41 @@
   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  = Sources.getFileManager();
+  llvm::ErrorOr FileEntryPtr =
+  FileMgr.getFile(AssumedFileName);
 
   unsigned Errors = 0;
-  DiagsBuffer->BeginSourceFile(LangOptions(), nullptr);
   if (WarnFormat && !NoWarnFormat) {
+ArrayRef> Ranges;
 for (const auto  : 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, 0);
+  SourceLocation NextLineBegin =
+  Sources.translateFileLineCol(FileEntryPtr.get(), PLoc.getLine(), 0);
+
+  const char *StartBuf = Sources.getCharacterData(LineBegin);
+  const char *EndBuf = Sources.getCharacterData(NextLineBegin);
+
+  StringRef Line(StartBuf, (EndBuf - StartBuf));
+
+  SMDiagnostic Diags(
+  llvm::SourceMgr(), SMLoc(), AssumedFileName, PLoc.getLine(),
+  PLoc.getColumn() - 1,
+  WarningsAsErrors ? SourceMgr::DiagKind::DK_Error
+   : SourceMgr::DiagKind::DK_Warning,
+  "code should be clang-formatted [-Wclang-format-violations]", Line,
+  ArrayRef>());
+
+  Diags.print(nullptr, llvm::errs(), (ShowColors && !NoShowColors));
   Errors++;
   if (ErrorLimit && Errors >= ErrorLimit)
 break;
 }
   }
-  DiagsBuffer->EndSourceFile();
   return WarningsAsErrors;
 }
 
Index: clang/tools/clang-format/CMakeLists.txt
===
--- clang/tools/clang-format/CMakeLists.txt
+++ clang/tools/clang-format/CMakeLists.txt
@@ -7,7 +7,6 @@
 set(CLANG_FORMAT_LIB_DEPS
   clangBasic
   clangFormat
-  clangFrontend
   clangRewrite
   clangToolingCore
   )


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -18,8 +18,8 @@
 #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/Demangle/StringView.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/InitLLVM.h"
@@ -325,12 +325,9 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowColors = (ShowColors && !NoShowColors);
 
-  TextDiagnosticPrinter *DiagsBuffer =

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-16 Thread Manuel Klimek via Phabricator via cfe-commits 
klimek added a comment.

In D68969#1709696 , @MyDeveloperDay 
wrote:

> In D68969#1709321 , @klimek wrote:
>
> > My intuitive solution would have been to get the char* for the start and 
> > end-location and then search forward and backwards for \n.
>
>
> I may need to, it feels slow for large files which I suspect is in the 
> splitting, which will be mostly unnecessary.


It's a weird trade-off: for lots of lines, the inside-out algorithm will be 
better, but if we have a very very long line with lots of warnings, the 
inside-out algorithm will be quadratic.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-15 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

In D68969#1709321 , @klimek wrote:

> My intuitive solution would have been to get the char* for the start and 
> end-location and then search forward and backwards for \n.


I may need to, it feels slow for large files which I suspect is in the 
splitting, which will be mostly unnecessary.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-15 Thread Manuel Klimek via Phabricator via cfe-commits 
klimek added a comment.

My intuitive solution would have been to get the char* for the start and 
end-location and then search forward and backwards for \n.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68969/new/

https://reviews.llvm.org/D68969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68969: [clang-format] Remove the dependency on frontend

2019-10-14 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: thakis, klimek, mitchell-stellar.
MyDeveloperDay added projects: clang, clang-format.
Herald added a subscriber: mgorny.

Address review comments from D68554: [clang-format] Proposal for clang-format 
to give compiler style warnings  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.


Repository:
  rC Clang

https://reviews.llvm.org/D68969

Files:
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-format/ClangFormat.cpp


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ 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 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowColors = (ShowColors && !NoShowColors);
 
-  TextDiagnosticPrinter *DiagsBuffer =
-  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
-
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
   IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
+  new DiagnosticsEngine(DiagID, &*DiagOpts));
 
   IntrusiveRefCntPtr InMemoryFileSystem(
   new llvm::vfs::InMemoryFileSystem);
@@ -339,24 +335,35 @@
   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]");
+  SmallVector Lines;
+  Code->getBuffer().split(Lines, "\n", /*MaxSplit=*/-1,
+  /*KeepEmpty=*/true);
 
   unsigned Errors = 0;
-  DiagsBuffer->BeginSourceFile(LangOptions(), nullptr);
   if (WarnFormat && !NoWarnFormat) {
+ArrayRef> Ranges;
 for (const auto  : Replaces) {
-  Diags->Report(
-  Sources.getLocForStartOfFile(FileID).getLocWithOffset(R.getOffset()),
-  ID);
+  PresumedLoc PLoc = Sources.getPresumedLoc(
+  
Sources.getLocForStartOfFile(FileID).getLocWithOffset(R.getOffset()));
+
+  StringRef Line;
+  if (PLoc.getLine() < Lines.size() && PLoc.getLine() > 0)
+Line = Lines[PLoc.getLine() - 1];
+
+  SMDiagnostic Diags(
+  llvm::SourceMgr(), SMLoc(), AssumedFileName, PLoc.getLine(),
+  PLoc.getColumn() - 1,
+  WarningsAsErrors ? SourceMgr::DiagKind::DK_Error
+   : SourceMgr::DiagKind::DK_Warning,
+  "code should be clang-formatted [-Wclang-format-violations]", Line,
+  ArrayRef>());
+
+  Diags.print(nullptr, llvm::errs(), (ShowColors && !NoShowColors));
   Errors++;
   if (ErrorLimit && Errors >= ErrorLimit)
 break;
 }
   }
-  DiagsBuffer->EndSourceFile();
   return WarningsAsErrors;
 }
 
Index: clang/tools/clang-format/CMakeLists.txt
===
--- clang/tools/clang-format/CMakeLists.txt
+++ clang/tools/clang-format/CMakeLists.txt
@@ -7,7 +7,6 @@
 set(CLANG_FORMAT_LIB_DEPS
   clangBasic
   clangFormat
-  clangFrontend
   clangRewrite
   clangToolingCore
   )


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ 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 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowColors = (ShowColors && !NoShowColors);
 
-  TextDiagnosticPrinter *DiagsBuffer =
-  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
-
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
   IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
+  new DiagnosticsEngine(DiagID, &*DiagOpts));
 
   IntrusiveRefCntPtr InMemoryFileSystem(
   new llvm::vfs::InMemoryFileSystem);
@@ -339,24 +335,35 @@