Implement soft reset of the diagnostics engine.

[tapaswenipathak]

Summary

Implement soft reset of the diagnostics engine.

This patch implements soft reset and adds tests for soft reset success of the
diagnostics engine.

This patch adds unittest for soft reset success using #define public
private. Multiple options were used before arriving on using #define public
private as discussed in https://github.com/vgvassilev/clang/pull/1.

1. Enable getter for private variables distinguishing soft reset.
2. Using any existing locs
3. gtest:friend_test https://google.github.io/googletest/reference/testing.html#FRIEND_TEST
4. using #define public private accomapnied with #undef private before
and after the inclusion of `.h` file to access a few private members of
enclosing class in the test function.

Finally, a test helper friend function is added to test soft reset
success.

This patch also adds a clang-repl test and adapts clang:tools:clang-repl to
report unsuccess when running with -verify.

Screenshots of testing this patch can be found on the mentioned pull
request.

Co-authored-by: Vassil Vassilev <[e-mail]>

[rsmith]

I don't think we can live with the #define private public approach, not least because this violates the ODR and might lead to compile failures using modules as a result. As an alternative, how about:

• Adding a friend void DiagnosticTestHelper(); declaration to DiagnosticsEngine
• Defining that function in DiagnosticTest.cpp and calling it to do your checks of the diagnostics engine's state.

[tapaswenipathak]

Addresses review comment by @rsmith: https://reviews.llvm.org/D126183#3534846.

[tapaswenipathak]

runs clang-format on the diff.

[tapaswenipathak]

Fixes: https://buildkite.com/llvm-project/diff-checks/builds/106655#01810a71-0b5c-4b92-9d05-22a1ed950833

.

[tapaswenipathak]

Fixes: https://buildkite.com/llvm-project/premerge-checks/builds/95001#01810b0b-6313-400f-aaf0-35855916ec93

I pasted the wrong diff. sorry!

(have multiple build repository in local)

[tapaswenipathak]

This comment was removed by tapaswenipathak.

[tapaswenipathak]

this should be green. :)

[v.g.vassilev]

This looks good to me, modulo the inline comment.

Let's wait for @rsmith's green light.

clang/lib/Interpreter/IncrementalParser.cpp

201 ↗

(On Diff #433362)

Diags.Reset(/*soft=*/true);

[v.g.vassilev]

clang/lib/Interpreter/IncrementalParser.cpp

200 ↗

(On Diff #433362)

We should remove this fixme, too.

[tapaswenipathak]

Addresses review comments by @v.g.vassilev.

[mstorsjo]

FYI, this change caused warnings when built with GCC:

[1/1] Building CXX object tools/clang/unittests/Basic/CMakeFiles/BasicTests.dir/DiagnosticTest.cpp.o
../tools/clang/unittests/Basic/DiagnosticTest.cpp:17:6: warning: ‘void clang::DiagnosticsTestHelper(clang::DiagnosticsEngine&)’ has not been declared within ‘clang’
   17 | void clang::DiagnosticsTestHelper(DiagnosticsEngine &diag) {
      |      ^~~~~
In file included from ../tools/clang/unittests/Basic/DiagnosticTest.cpp:9:
../tools/clang/include/clang/Basic/Diagnostic.h:548:15: note: only here as a ‘friend’
  548 |   friend void DiagnosticsTestHelper(DiagnosticsEngine &);
      |               ^~~~~~~~~~~~~~~~~~~~~

clang/include/clang/Basic/Diagnostic.h

   DiagnosticsEngine &operator=(const DiagnosticsEngine &) = delete;
   ~DiagnosticsEngine();
 
+  friend void DiagnosticsTestHelper(DiagnosticsEngine &);
   LLVM_DUMP_METHOD void dump() const;
   LLVM_DUMP_METHOD void dump(StringRef DiagName) const;
 

     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.

clang/lib/Basic/Diagnostic.cpp

   return true;
 }
 
-void DiagnosticsEngine::Reset() {
+void DiagnosticsEngine::Reset(bool soft /*=false*/) {
   ErrorOccurred = false;
   UncompilableErrorOccurred = false;
   FatalErrorOccurred = false;

   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());
+  }
 }
 
 void DiagnosticsEngine::SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1,

clang/test/Interpreter/error-recovery-pragmas.cpp

+// RUN: cat %s | clang-repl -Xcc -Xclang -Xcc -verify
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wmultichar"
+// Reset should not delete #pragmas
+error; // expected-error {{use of undeclared identifier}}
+void no_diag_multichar(void) { char c = (char)'ab'; }
+quit

clang/tools/clang-repl/ClangRepl.cpp

       llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
   }
 
+  bool hadErrors = false;
   if (OptInputs.empty()) {
     llvm::LineEditor LE("clang-repl");
     // FIXME: Add LE.setListCompleter
     while (llvm::Optional<std::string> Line = LE.readLine()) {
       if (*Line == "quit")
         break;
-      if (auto Err = Interp->ParseAndExecute(*Line))
+      if (auto Err = Interp->ParseAndExecute(*Line)) {
         llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
+        hadErrors = true;
+      }
     }
   }
 

 
   llvm::llvm_shutdown();
 
-  return 0;
+  return hadErrors;
 }

clang/unittests/Basic/DiagnosticTest.cpp

 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.

     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 differentiating soft reset
+  DiagnosticsTestHelper(Diags);
+
+  EXPECT_FALSE(Diags.isDiagnosticInFlight());
+  EXPECT_TRUE(Diags.isLastDiagnosticIgnored());
+}
+
 TEST(DiagnosticTest, diagnosticError) {
   DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
                           new IgnoringDiagConsumer());