diff --git a/lib/CppInterOp/Compatibility.h b/lib/CppInterOp/Compatibility.h index 56319b5d0..11db3054a 100644 --- a/lib/CppInterOp/Compatibility.h +++ b/lib/CppInterOp/Compatibility.h @@ -7,6 +7,7 @@ #include "clang/AST/DeclTemplate.h" #include "clang/AST/GlobalDecl.h" +#include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/DiagnosticOptions.h" #if CLANG_VERSION_MAJOR < 21 @@ -27,11 +28,13 @@ #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/FileSystem.h" +#include "llvm/Support/raw_ostream.h" #include #include #include #include +#include #ifdef _MSC_VER #define dup _dup @@ -51,6 +54,57 @@ 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 = false; + clang::DiagnosticsEngine& m_diags; + // clang::DiagnosticConsumer* m_oldClient = nullptr; + // bool m_oldShouldOwn = false; + // clang::IgnoringDiagConsumer m_ignoring_consumer; + bool m_oldSuppressDiags; + +public: + explicit DiagnosticGuard(clang::DiagnosticsEngine& diags, bool silent) + : m_is_silent(silent), m_diags(diags), + m_oldSuppressDiags(diags.getSuppressAllDiagnostics()) { + m_diags.setSuppressAllDiagnostics(silent); + // 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() { + m_diags.setSuppressAllDiagnostics(m_oldSuppressDiags); + // // 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(const DiagnosticGuard&&) = delete; + DiagnosticGuard& operator=(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) const { + 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 diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index 9c6f00432..a4da139ca 100644 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/lib/CppInterOp/CppInterOp.cpp @@ -3606,30 +3606,13 @@ void GetIncludePaths(std::vector& 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) { diff --git a/lib/CppInterOp/CppInterOpInterpreter.h b/lib/CppInterOp/CppInterOpInterpreter.h index d4cd3d501..ebea392c7 100644 --- a/lib/CppInterOp/CppInterOpInterpreter.h +++ b/lib/CppInterOp/CppInterOpInterpreter.h @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -389,21 +390,23 @@ 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, /*V=*/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; } @@ -411,8 +414,7 @@ class Interpreter { *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; diff --git a/unittests/CMakeLists.txt b/unittests/CMakeLists.txt index 0359d718a..d4b21ba7c 100644 --- a/unittests/CMakeLists.txt +++ b/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}) diff --git a/unittests/CppInterOp/InterpreterTest.cpp b/unittests/CppInterOp/InterpreterTest.cpp index 51652a5c6..bba3e4217 100644 --- a/unittests/CppInterOp/InterpreterTest.cpp +++ b/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); diff --git a/unittests/CppInterOp/Utils.cpp b/unittests/CppInterOp/Utils.cpp index 57489ae0a..a8284b981 100644 --- a/unittests/CppInterOp/Utils.cpp +++ b/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;