[clang] 946c45a - Implement soft reset of the diagnostics engine.

[Vassil Vassilev via cfe-commits]

Author: Tapasweni Pathak
Date: 2022-06-24T14:46:54Z
New Revision: 946c45a4ed5d5e2f262110a27390369f0d8fc3eb

URL: https://github.com/llvm/llvm-project/commit/946c45a4ed5d5e2f262110a27390369f0d8fc3eb
DIFF: https://github.com/llvm/llvm-project/commit/946c45a4ed5d5e2f262110a27390369f0d8fc3eb.diff

LOG: Implement soft reset of the diagnostics engine.

This patch implements soft reset and adds tests for soft reset success of the
diagnostics engine. This allows us to recover from errors in clang-repl without
resetting the pragma handlers' state.

Differential revision: https://reviews.llvm.org/D126183

Added: 
    

Modified: 
    clang/include/clang/Basic/Diagnostic.h
    clang/lib/Basic/Diagnostic.cpp
    clang/lib/Interpreter/IncrementalParser.cpp
    clang/tools/clang-repl/ClangRepl.cpp
    clang/unittests/Basic/DiagnosticTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index 33ad0827c0ca5..09857da61d326 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -545,6 +545,7 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
   DiagnosticsEngine &operator=(const DiagnosticsEngine &) = delete;
   ~DiagnosticsEngine();
 
+  friend void DiagnosticsTestHelper(DiagnosticsEngine &);
   LLVM_DUMP_METHOD void dump() const;
   LLVM_DUMP_METHOD void dump(StringRef DiagName) const;
 
@@ -891,9 +892,9 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
     LastDiagLevel = Other.LastDiagLevel;
   }
 
-  /// Reset the state of the diagnostic object to its initial
-  /// configuration.
-  void Reset();
+  /// Reset the state of the diagnostic object to its initial configuration.
+  /// \param[in] soft - if true, doesn't reset the diagnostic mappings and state
+  void Reset(bool soft = false);
 
   //===--------------------------------------------------------------------===//
   // DiagnosticsEngine classification and reporting interfaces.

diff  --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index d14134f99ee95..deb398756e373 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -130,7 +130,7 @@ bool DiagnosticsEngine::popMappings(SourceLocation Loc) {
   return true;
 }
 
-void DiagnosticsEngine::Reset() {
+void DiagnosticsEngine::Reset(bool soft /*=false*/) {
   ErrorOccurred = false;
   UncompilableErrorOccurred = false;
   FatalErrorOccurred = false;
@@ -145,15 +145,17 @@ void DiagnosticsEngine::Reset() {
   LastDiagLevel = DiagnosticIDs::Ignored;
   DelayedDiagID = 0;
 
-  // Clear state related to #pragma diagnostic.
-  DiagStates.clear();
-  DiagStatesByLoc.clear();
-  DiagStateOnPushStack.clear();
+  if (!soft) {
+    // Clear state related to #pragma diagnostic.
+    DiagStates.clear();
+    DiagStatesByLoc.clear();
+    DiagStateOnPushStack.clear();
 
-  // Create a DiagState and DiagStatePoint representing diagnostic changes
-  // through command-line.
-  DiagStates.emplace_back();
-  DiagStatesByLoc.appendFirst(&DiagStates.back());
+    // Create a DiagState and DiagStatePoint representing diagnostic changes
+    // through command-line.
+    DiagStates.emplace_back();
+    DiagStatesByLoc.appendFirst(&DiagStates.back());
+  }
 }
 
 void DiagnosticsEngine::SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1,

diff  --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp
index 122cf7b065bb0..42cda512a8ae3 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -203,8 +203,8 @@ IncrementalParser::ParseOrWrapTopLevelDecl() {
       }
     }
 
-    // FIXME: Do not reset the pragma handlers.
-    Diags.Reset();
+    Diags.Reset(/*soft=*/true);
+    Diags.getClient()->clear();
     return llvm::make_error<llvm::StringError>("Parsing failed.",
                                                std::error_code());
   }

diff  --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp
index 088615e30a2d6..ca784ad0c07de 100644
--- a/clang/tools/clang-repl/ClangRepl.cpp
+++ b/clang/tools/clang-repl/ClangRepl.cpp
@@ -48,6 +48,23 @@ static void LLVMErrorHandler(void *UserData, const char *Message,
   exit(GenCrashDiag ? 70 : 1);
 }
 
+// If we are running with -verify a reported has to be returned as unsuccess.
+// This is relevant especially for the test suite.
+static int checkDiagErrors(const clang::CompilerInstance *CI) {
+  unsigned Errs = CI->getDiagnostics().getClient()->getNumErrors();
+  if (CI->getDiagnosticOpts().VerifyDiagnostics) {
+    // If there was an error that came from the verifier we must return 1 as
+    // an exit code for the process. This will make the test fail as expected.
+    clang::DiagnosticConsumer *Client = CI->getDiagnostics().getClient();
+    Client->EndSourceFile();
+    Errs = Client->getNumErrors();
+
+    // The interpreter expects BeginSourceFile/EndSourceFiles to be balanced.
+    Client->BeginSourceFile(CI->getLangOpts(), &CI->getPreprocessor());
+  }
+  return Errs ? EXIT_FAILURE : EXIT_SUCCESS;
+}
+
 llvm::ExitOnError ExitOnErr;
 int main(int argc, const char **argv) {
   ExitOnErr.setBanner("clang-repl: ");
@@ -106,5 +123,5 @@ int main(int argc, const char **argv) {
 
   llvm::llvm_shutdown();
 
-  return 0;
+  return checkDiagErrors(Interp->getCompilerInstance());
 }

diff  --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp
index 8d018a8b9cef4..0ac931650e524 100644
--- a/clang/unittests/Basic/DiagnosticTest.cpp
+++ b/clang/unittests/Basic/DiagnosticTest.cpp
@@ -14,6 +14,15 @@
 using namespace llvm;
 using namespace clang;
 
+void clang::DiagnosticsTestHelper(DiagnosticsEngine &diag) {
+  unsigned delayedDiagID = 0U;
+
+  EXPECT_EQ(diag.DelayedDiagID, delayedDiagID);
+  EXPECT_FALSE(diag.DiagStates.empty());
+  EXPECT_TRUE(diag.DiagStatesByLoc.empty());
+  EXPECT_TRUE(diag.DiagStateOnPushStack.empty());
+}
+
 namespace {
 
 // Check that DiagnosticErrorTrap works with SuppressAllDiagnostics.
@@ -71,6 +80,32 @@ TEST(DiagnosticTest, fatalsAsError) {
     EXPECT_EQ(Diags.getNumWarnings(), FatalsAsError);
   }
 }
+
+// Check that soft RESET works as intended
+TEST(DiagnosticTest, softReset) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
+                          new IgnoringDiagConsumer());
+
+  unsigned numWarnings = 0U, numErrors = 0U;
+
+  Diags.Reset(true);
+  // Check For ErrorOccurred and TrapNumErrorsOccurred
+  EXPECT_FALSE(Diags.hasErrorOccurred());
+  EXPECT_FALSE(Diags.hasFatalErrorOccurred());
+  EXPECT_FALSE(Diags.hasUncompilableErrorOccurred());
+  // Check for UnrecoverableErrorOccurred and TrapNumUnrecoverableErrorsOccurred
+  EXPECT_FALSE(Diags.hasUnrecoverableErrorOccurred());
+
+  EXPECT_EQ(Diags.getNumWarnings(), numWarnings);
+  EXPECT_EQ(Diags.getNumErrors(), numErrors);
+
+  // Check for private variables of DiagnosticsEngine 
diff erentiating soft reset
+  DiagnosticsTestHelper(Diags);
+
+  EXPECT_FALSE(Diags.isDiagnosticInFlight());
+  EXPECT_TRUE(Diags.isLastDiagnosticIgnored());
+}
+
 TEST(DiagnosticTest, diagnosticError) {
   DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
                           new IgnoringDiagConsumer());