Provide FIX-IT notes to describe what fix-it is doing behind the

scenes, using the underlying diagnostic client to format the
messages.

llvm-svn: 68324

clang/include/clang/Basic/Diagnostic.h

@@ -299,6 +299,9 @@ public:
   /// @c Pos represents the source location associated with the diagnostic,
   /// which can be an invalid location if no position information is available.
   inline DiagnosticBuilder Report(FullSourceLoc Pos, unsigned DiagID);
+
+  /// \brief Clear out the current diagnostic.
+  void Clear() { CurDiagID = ~0U; }
   
 private:
   /// getDiagnosticLevel - This is an internal implementation helper used when
@@ -423,8 +426,8 @@ public:
     // DiagnosticClient.
     DiagObj->ProcessDiag();
 
-    // This diagnostic is no longer in flight.
-    DiagObj->CurDiagID = ~0U;
+    // Clear out the current diagnostic object.
+    DiagObj->Clear();
 
     // This diagnostic is dead.
     DiagObj = 0;

clang/include/clang/Basic/DiagnosticFrontendKinds.td

@@ -14,4 +14,12 @@ def err_fe_unknown_triple : Error<
 def err_fe_error_reading : Error<"error reading '%0'">;
 def err_fe_error_reading_stdin : Error<"error reading stdin">;
 
+def note_fixit_applied : Note<"FIX-IT applied suggested code changes">;
+def note_fixit_in_macro : Note<
+    "FIX-IT unable to apply suggested code changes in a macro">;
+def note_fixit_failed : Note<
+    "FIX-IT unable to apply suggested code changes">;
+def note_fixit_unfixed_error : Note<"FIX-IT detected an error it cannot fix">;
+def warn_fixit_no_changes : Note<
+    "FIX-IT detected errors it could not fix; no output will be generated">;
 }

clang/include/clang/Frontend/FixItRewriter.h

@@ -16,27 +16,30 @@
 #define LLVM_CLANG_FRONTEND_FIX_IT_REWRITER_H
 
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Rewrite/Rewriter.h"
 
 namespace clang {
 
-class Rewriter;
 class SourceManager;
 
 class FixItRewriter : public DiagnosticClient {
-  /// \brief The adapted diagnostic client, to which we will forward
-  /// any diagnostics.
-  DiagnosticClient *Client;
+  /// \brief The diagnostics machinery.
+  Diagnostic &Diags;
 
   /// \brief The rewriter used to perform the various code
   /// modifications.
-  Rewriter *Rewrite;
+  Rewriter Rewrite;
+
+  /// \brief The diagnostic client that performs the actual formatting
+  /// of error messages.
+  DiagnosticClient *Client;
 
   /// \brief The number of rewriter failures.
   unsigned NumFailures;
 
 public:
   /// \brief Initialize a new fix-it rewriter.
-  FixItRewriter(DiagnosticClient *Client, SourceManager &SourceMgr);
+  FixItRewriter(Diagnostic &Diags, SourceManager &SourceMgr);
 
   /// \brief Destroy the fix-it rewriter.
   ~FixItRewriter();
@@ -58,6 +61,8 @@ public:
   virtual void HandleDiagnostic(Diagnostic::Level DiagLevel,
                                 const DiagnosticInfo &Info);
 
+  /// \brief Emit a diagnostic via the adapted diagnostic client.
+  void Diag(FullSourceLoc Loc, unsigned DiagID);
 };
 
 }

clang/lib/Frontend/FixItRewriter.cpp

@@ -15,31 +15,27 @@
 
 #include "clang/Frontend/FixItRewriter.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Rewrite/Rewriter.h"
+#include "clang/Frontend/FrontendDiagnostic.h"
 #include "llvm/ADT/OwningPtr.h"
 #include "llvm/Support/Streams.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/System/Path.h"
-#include <cstdio>
 using namespace clang;
 
-FixItRewriter::FixItRewriter(DiagnosticClient *Client, 
-                             SourceManager &SourceMgr)
-  : Client(Client), NumFailures(0) {
-  Rewrite = new Rewriter(SourceMgr);
+FixItRewriter::FixItRewriter(Diagnostic &Diags, SourceManager &SourceMgr)
+  : Diags(Diags), Rewrite(SourceMgr), NumFailures(0) {
+  Client = Diags.getClient();
+  Diags.setClient(this);
 }
 
 FixItRewriter::~FixItRewriter() {
-  delete Rewrite;
+  Diags.setClient(Client);
 }
 
 bool FixItRewriter::WriteFixedFile(const std::string &InFileName, 
                                    const std::string &OutFileName) {
   if (NumFailures > 0) {
-    // FIXME: Use diagnostic machinery!
-    std::fprintf(stderr, 
-                 "%d fix-it failures detected; code will not be modified\n",
-                 NumFailures);
+    Diag(FullSourceLoc(), diag::warn_fixit_no_changes);
     return true;
   }
 
@@ -67,9 +63,9 @@ bool FixItRewriter::WriteFixedFile(const std::string &InFileName,
     OwnedStream.reset(OutFile);
   }  
 
-  FileID MainFileID = Rewrite->getSourceMgr().getMainFileID();
+  FileID MainFileID = Rewrite.getSourceMgr().getMainFileID();
   if (const RewriteBuffer *RewriteBuf = 
-        Rewrite->getRewriteBufferFor(MainFileID)) {
+        Rewrite.getRewriteBufferFor(MainFileID)) {
     *OutFile << std::string(RewriteBuf->begin(), RewriteBuf->end());
   } else {
     std::fprintf(stderr, "Main file is unchanged\n");
@@ -95,30 +91,26 @@ void FixItRewriter::HandleDiagnostic(Diagnostic::Level DiagLevel,
        Idx < Last; ++Idx) {
     const CodeModificationHint &Hint = Info.getCodeModificationHint(Idx);
     if (Hint.RemoveRange.isValid() &&
-        Rewrite->getRangeSize(Hint.RemoveRange) == -1) {
+        Rewrite.getRangeSize(Hint.RemoveRange) == -1) {
       CanRewrite = false;
       break;
     }
 
     if (Hint.InsertionLoc.isValid() && 
-        !Rewrite->isRewritable(Hint.InsertionLoc)) {
+        !Rewrite.isRewritable(Hint.InsertionLoc)) {
       CanRewrite = false;
       break;
     }
   }
 
   if (!CanRewrite) { 
-    if (Info.getNumCodeModificationHints() > 0) {
-      // FIXME: warn the user that this rewrite couldn't be done
-    }
+    if (Info.getNumCodeModificationHints() > 0)
+      Diag(Info.getLocation(), diag::note_fixit_in_macro);
 
     // If this was an error, refuse to perform any rewriting.
     if (DiagLevel == Diagnostic::Error || DiagLevel == Diagnostic::Fatal) {
-      if (++NumFailures == 1) {
-        // FIXME: use diagnostic machinery to print this.
-        std::fprintf(stderr, "error without fix-it advice detected; "
-                     "fix-it will produce no output\n");
-      }
+      if (++NumFailures == 1)
+        Diag(Info.getLocation(), diag::note_fixit_unfixed_error);
     }
     return;
   }
@@ -129,27 +121,43 @@ void FixItRewriter::HandleDiagnostic(Diagnostic::Level DiagLevel,
     const CodeModificationHint &Hint = Info.getCodeModificationHint(Idx);
     if (!Hint.RemoveRange.isValid()) {
       // We're adding code.
-      if (Rewrite->InsertStrBefore(Hint.InsertionLoc, Hint.CodeToInsert))
+      if (Rewrite.InsertStrBefore(Hint.InsertionLoc, Hint.CodeToInsert))
         Failed = true;
       continue;
     }
     
     if (Hint.CodeToInsert.empty()) {
       // We're removing code.
-      if (Rewrite->RemoveText(Hint.RemoveRange.getBegin(),
-                              Rewrite->getRangeSize(Hint.RemoveRange)))
+      if (Rewrite.RemoveText(Hint.RemoveRange.getBegin(),
+                             Rewrite.getRangeSize(Hint.RemoveRange)))
         Failed = true;
       continue;
     } 
       
     // We're replacing code.
-    if (Rewrite->ReplaceText(Hint.RemoveRange.getBegin(),
-                             Rewrite->getRangeSize(Hint.RemoveRange),
-                             Hint.CodeToInsert.c_str(),
-                             Hint.CodeToInsert.size()))
+    if (Rewrite.ReplaceText(Hint.RemoveRange.getBegin(),
+                            Rewrite.getRangeSize(Hint.RemoveRange),
+                            Hint.CodeToInsert.c_str(),
+                            Hint.CodeToInsert.size()))
       Failed = true;
   }
 
-  if (Failed) // FIXME: notify the user that the rewrite failed.
+  if (Failed) {
     ++NumFailures;
+    Diag(Info.getLocation(), diag::note_fixit_failed);
+    return;
+  }
+
+  Diag(Info.getLocation(), diag::note_fixit_applied);
+}
+
+/// \brief Emit a diagnostic via the adapted diagnostic client.
+void FixItRewriter::Diag(FullSourceLoc Loc, unsigned DiagID) {
+  // When producing this diagnostic, we temporarily bypass ourselves,
+  // clear out any current diagnostic, and let the downstream client
+  // format the diagnostic.
+  Diags.setClient(Client);
+  Diags.Clear();
+  Diags.Report(Loc, DiagID);
+  Diags.setClient(this);  
 }

clang/tools/clang-cc/clang-cc.cpp

@@ -1452,9 +1452,8 @@ static void ProcessInputFile(Preprocessor &PP, PreprocessorFactory &PPF,
   case FixIt:
     llvm::TimeRegion Timer(ClangFrontendTimer);
     Consumer.reset(new ASTConsumer());
-    FixItRewrite = new FixItRewriter(PP.getDiagnostics().getClient(),
+    FixItRewrite = new FixItRewriter(PP.getDiagnostics(),
                                      PP.getSourceManager());
-    PP.getDiagnostics().setClient(FixItRewrite);
     break;
   }