[PATCH] D111815: [clang-format] [PR42014,PR52021] don't let clang-format assert/crash when file being formatted is read-only/locked

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: thakis, curdeius, HazardyKnusperkeks.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

This is a bug which gets reported from time to time and we've had multiple 
attempts to fix it, but don't want to fix it by adding frontEnd to the mix.

D68554: [clang-format] Proposal for clang-format to give compiler style 
warnings <https://reviews.llvm.org/D68554>
D90121: clang-format: Add a consumer to diagnostics engine 
<https://reviews.llvm.org/D90121>
D69854: [clang-format] [RELAND] Remove the dependency on frontend 
<https://reviews.llvm.org/D69854>

This patch aim to find a trivial, but not that sophisticated way of emitting 
the error without the additional impact of adding libFrontEnd to clang-format.

See D90121: clang-format: Add a consumer to diagnostics engine 
<https://reviews.llvm.org/D90121> for analysis of why we don't want those 
previous attempts

We can now see this:

  $ clang-format -i test1.cpp
  clang-format error:unable to rename temporary 'test1.cpp-4e5b1ac5' to output 
file 'test1.cpp': 'permission denied'

Rather than

  Assertion failed: getClient() && "DiagnosticClient not set!", file 
C:\llvm-project\clang\lib\Basic\Diagnostic.cpp, line 522
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace.
  Stack dump:
  0.      Program arguments: C:\\build_ninja\\bin\\clang-format.exe -i test1.cpp
   #0 0x00007ff63cd7244c (C:\build_ninja\bin\clang-format.exe+0x12244c)
   #1 0x00007fff3db28e05 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0xa8e05)
  ...
   #6 0x00007fff3db3158f (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0xb158f)
   #7 0x00007ff63ce6d4ff (C:\build_ninja\bin\clang-format.exe+0x21d4ff)
   #8 0x00007ff63ce7dcf6 (C:\build_ninja\bin\clang-format.exe+0x22dcf6)
  ....
  #17 0x00007ff63d16457e (C:\build_ninja\bin\clang-format.exe+0x51457e)
  #18 0x00007fff9b7f7034 (C:\WINDOWS\System32\KERNEL32.DLL+0x17034)
  #19 0x00007fff9ce02651 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x52651)

Not sure how I can easily unit test this, as I had to use Excel to lock the 
file!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111815

Files:
  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
@@ -366,6 +366,18 @@
   outs() << "</replacements>\n";
 }
 
+class ClangFormatDiagConsumer : public DiagnosticConsumer {
+  virtual void anchor() {}
+
+  void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+                        const Diagnostic &Info) override {
+
+    SmallVector<char, 16> vec;
+    Info.FormatDiagnostic(vec);
+    errs() << "clang-format error:" << vec << "\n";
+  }
+};
+
 // Returns true on error.
 static bool format(StringRef FileName) {
   if (!OutputXML && Inplace && FileName == "-") {
@@ -477,9 +489,12 @@
     IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
         new llvm::vfs::InMemoryFileSystem);
     FileManager Files(FileSystemOptions(), InMemoryFileSystem);
+
+    IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions());
+    ClangFormatDiagConsumer IgnoreDiagnostics;
     DiagnosticsEngine Diagnostics(
-        IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs),
-        new DiagnosticOptions);
+        IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs), &*DiagOpts,
+        &IgnoreDiagnostics, false);
     SourceManager Sources(Diagnostics, Files);
     FileID ID = createInMemoryFile(AssumedFileName, *Code, Sources, Files,
                                    InMemoryFileSystem.get());



Index: clang/tools/clang-format/ClangFormat.cpp
===================================================================
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -366,6 +366,18 @@
   outs() << "</replacements>\n";
 }
 
+class ClangFormatDiagConsumer : public DiagnosticConsumer {
+  virtual void anchor() {}
+
+  void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+                        const Diagnostic &Info) override {
+
+    SmallVector<char, 16> vec;
+    Info.FormatDiagnostic(vec);
+    errs() << "clang-format error:" << vec << "\n";
+  }
+};
+
 // Returns true on error.
 static bool format(StringRef FileName) {
   if (!OutputXML && Inplace && FileName == "-") {
@@ -477,9 +489,12 @@
     IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
         new llvm::vfs::InMemoryFileSystem);
     FileManager Files(FileSystemOptions(), InMemoryFileSystem);
+
+    IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions());
+    ClangFormatDiagConsumer IgnoreDiagnostics;
     DiagnosticsEngine Diagnostics(
-        IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs),
-        new DiagnosticOptions);
+        IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs), &*DiagOpts,
+        &IgnoreDiagnostics, false);
     SourceManager Sources(Diagnostics, Files);
     FileID ID = createInMemoryFile(AssumedFileName, *Code, Sources, Files,
                                    InMemoryFileSystem.get());

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