When Cpp::Declare is silent the API should retain the error code.

Author: vgvassilev

lib/CppInterOp/Compatibility.h

@@ -51,6 +51,51 @@ static inline char* GetEnv(const char* Var_Name) {
 #endif
 }
 
+/// A very lightweight RAII switcher. It manages diagnostic silencing
+/// and provides a unified interface for handling LLVM Errors.
+class DiagnosticGuard {
+  bool m_is_silent;
+  clang::DiagnosticsEngine& m_diags;
+  clang::DiagnosticConsumer* m_oldClient;
+  bool m_oldShouldOwn;
+  clang::IgnoringDiagConsumer m_ignoring_consumer;
+
+public:
+  explicit DiagnosticGuard(clang::DiagnosticsEngine& diags, bool silent)
+      : m_is_silent(silent), m_diags(diags) {
+    if (m_is_silent) {
+      m_oldShouldOwn = m_diags.ownsClient();
+      if (m_oldShouldOwn)
+        m_oldClient = m_diags.takeClient().release();
+      else
+        m_oldClient = m_diags.getClient();
+      m_diags.setClient(&m_ignoring_consumer, /*ShouldOwnClient=*/false);
+    }
+  }
+
+  ~DiagnosticGuard() {
+    // Only restore if we actually swapped the client
+    if (m_is_silent) {
+      m_diags.setClient(m_oldClient, m_oldShouldOwn);
+    }
+  }
+
+  // Explicitly non-copyable
+  DiagnosticGuard(const DiagnosticGuard&) = delete;
+  DiagnosticGuard& operator=(const DiagnosticGuard&) = delete;
+
+  /// Consumes or logs the error based on the scope's silence setting.
+  void handleOrConsume(llvm::Error err, const char* msg) {
+    if (!err)
+      return; // Safety check for "success" errors
+
+    if (m_is_silent)
+      llvm::consumeError(std::move(err));
+    else
+      llvm::logAllUnhandledErrors(std::move(err), llvm::errs(), msg);
+  }
+};
+
 #if CLANG_VERSION_MAJOR < 21
 #define Print_Canonical_Types PrintCanonicalTypes
 #else

lib/CppInterOp/CppInterOp.cpp

@@ -3606,30 +3606,13 @@ void GetIncludePaths(std::vector<std::string>& IncludePaths, bool withSystem,
     IncludePaths.push_back(i);
 }
 
-namespace {
-
-class clangSilent {
-public:
-  clangSilent(clang::DiagnosticsEngine& diag) : fDiagEngine(diag) {
-    fOldDiagValue = fDiagEngine.getSuppressAllDiagnostics();
-    fDiagEngine.setSuppressAllDiagnostics(true);
-  }
-
-  ~clangSilent() { fDiagEngine.setSuppressAllDiagnostics(fOldDiagValue); }
-
-protected:
-  clang::DiagnosticsEngine& fDiagEngine;
-  bool fOldDiagValue;
-};
-} // namespace
-
 int Declare(compat::Interpreter& I, const char* code, bool silent) {
-  if (silent) {
-    clangSilent diagSuppr(I.getSema().getDiagnostics());
-    return I.declare(code);
-  }
-
+#ifdef CPPINTEROP_USE_CLING
+  DiagnosticGuard Guard(I.getSema().getDiagnostics(), silent);
   return I.declare(code);
+#else
+  return I.declare(code, silent);
+#endif // CPPINTEROP_USE_CLING
 }
 
 int Declare(const char* code, bool silent) {

lib/CppInterOp/CppInterOpInterpreter.h

@@ -389,30 +389,31 @@ class Interpreter {
     return nullptr;
   }
 
-  CompilationResult declare(const std::string& input,
+  CompilationResult declare(const std::string& input, bool silent = false,
                             clang::PartialTranslationUnit** PTU = nullptr) {
-    return process(input, /*Value=*/nullptr, PTU);
+    return process(input, /*Value=*/nullptr, silent, PTU);
   }
 
-  ///\brief Maybe transform the input line to implement cint command line
+  /// Maybe transform the input line to implement cint command line
   /// semantics (declarations are global) and compile to produce a module.
   ///
   CompilationResult process(const std::string& input, clang::Value* V = 0,
+                            bool silent = false,
                             clang::PartialTranslationUnit** PTU = nullptr,
                             bool disableValuePrinting = false) {
+    DiagnosticGuard Guard(getSema().getDiagnostics(), silent);
     auto PTUOrErr = Parse(input);
     if (!PTUOrErr) {
-      llvm::logAllUnhandledErrors(PTUOrErr.takeError(), llvm::errs(),
-                                  "Failed to parse via ::process:");
+      Guard.handleOrConsume(PTUOrErr.takeError(),
+                            "Failed to parse via ::process:");
       return Interpreter::kFailure;
     }
 
     if (PTU)
       *PTU = &*PTUOrErr;
 
     if (auto Err = Execute(*PTUOrErr)) {
-      llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
-                                  "Failed to execute via ::process:");
+      Guard.handleOrConsume(std::move(Err), "Failed to execute via ::process:");
       return Interpreter::kFailure;
     }
     return Interpreter::kSuccess;

unittests/CMakeLists.txt

@@ -37,6 +37,13 @@ function(add_cppinterop_unittest name)
     ${ARGN}
   )
   add_executable(${name} EXCLUDE_FROM_ALL ${ARG_UNPARSED_ARGUMENTS})
+  if(NOT LLVM_ENABLE_RTTI)
+    if(MSVC)
+      target_compile_options(${name} PRIVATE "/GR-")
+    else()
+      target_compile_options(${name} PRIVATE "-fno-rtti")
+    endif()
+  endif()
   add_dependencies(CppInterOpUnitTests ${name})
   target_include_directories(${name} PUBLIC ${CMAKE_CURRENT_BINARY_DIR} ${GTEST_INCLUDE_DIR})
   set_property(TARGET ${name} PROPERTY RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})

unittests/CppInterOp/InterpreterTest.cpp

@@ -200,6 +200,34 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, Interpreter_EmscriptenExceptionHandling) {
   EXPECT_TRUE(Cpp::Process(tryCatchCode) == 0);
 }
 
+TYPED_TEST(CPPINTEROP_TEST_MODE, Silent_Declare_Behavior) {
+  auto* I = TestFixture::CreateInterpreter();
+  ASSERT_TRUE(I);
+
+  EXPECT_FALSE(Cpp::Declare("int valid_var = 42;", /*silent=*/true));
+
+  // Should produce output
+  {
+    testing::internal::CaptureStderr();
+    // Intentionally broken code: "int x = ;"
+    bool success = Cpp::Declare("int broken_syntax = ;", /*silent=*/false);
+    std::string output = testing::internal::GetCapturedStderr();
+    EXPECT_TRUE(success);
+    EXPECT_FALSE(output.empty()) << "We expect to see a Clang error message\n";
+  }
+
+  // Should NOT produce output
+  {
+    testing::internal::CaptureStderr();
+    // Intentionally broken code
+    bool success = Cpp::Declare("invalid_type error_var = 0;", /*silent=*/true);
+    std::string output = testing::internal::GetCapturedStderr();
+    EXPECT_TRUE(success);
+    EXPECT_TRUE(output.empty())
+        << "Silent Declare leaked an error to stderr: " << output;
+  }
+}
+
 TYPED_TEST(CPPINTEROP_TEST_MODE, Interpreter_CreateInterpreter) {
   auto* I = TestFixture::CreateInterpreter();
   EXPECT_TRUE(I);

unittests/CppInterOp/Utils.cpp

@@ -76,7 +76,7 @@ void TestUtils::GetAllTopLevelDecls(
   }
 #else
   PartialTranslationUnit *T = nullptr;
-  Interp->process(code, /*Value*/nullptr, &T);
+  Interp->process(code, /*Value*/ nullptr, /*silent=*/false, &T);
   for (auto *D : T->TUPart->decls()) {
     if (filter_implicitGenerated && D->isImplicit())
       continue;