From 0d34ed1d29795991691e8c69faee58a1b8732960 Mon Sep 17 00:00:00 2001 From: Javier Lopez-Gomez Date: Thu, 4 Nov 2021 13:50:05 +0100 Subject: [PATCH 1/3] [tcling] Suppress -Wunused-result diagnostics in wrappers generated by TClingCallFunc A TClingCallFunc wrapper function might look as the excerpt below, where the function denoted by `func` may have been annotated as `[[nodiscard]]`. Note that if `ret == nullptr` the result of the call is unused. ``` extern "C" void __cf_0(void* obj, int nargs, void** args, void* ret) { if (ret) { new (ret) (return_type) ((class_name*)obj)->func(args...); } else { ((class_name*)obj)->func(args...); } } ``` In turn, this triggers warnings when used by cppyy/PyROOT, e.g. ``` >>> import ROOT >>> v = ROOT.std.vector(int)() >>> v.empty() input_line_34:10:7: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result] ((const vector*)obj)->empty(); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ True >>> ``` Given the above, the second call expression will be casted to `void`. Closes issue #8622. --- core/metacling/src/TClingCallFunc.cxx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/metacling/src/TClingCallFunc.cxx b/core/metacling/src/TClingCallFunc.cxx index 8bc0227b150a1..5e2135246964f 100644 --- a/core/metacling/src/TClingCallFunc.cxx +++ b/core/metacling/src/TClingCallFunc.cxx @@ -982,7 +982,7 @@ void TClingCallFunc::make_narg_call_with_return(const unsigned N, const string & // new (ret) (return_type) ((class_name*)obj)->func(args...); // } // else { - // ((class_name*)obj)->func(args...); + // (void)(((class_name*)obj)->func(args...)); // } // const FunctionDecl *FD = GetDecl(); @@ -1085,8 +1085,9 @@ void TClingCallFunc::make_narg_call_with_return(const unsigned N, const string & for (int i = 0; i < indent_level; ++i) { callbuf << kIndentString; } + callbuf << "(void)("; make_narg_call(type_name, N, typedefbuf, callbuf, class_name, indent_level); - callbuf << ";\n"; + callbuf << ");\n"; for (int i = 0; i < indent_level; ++i) { callbuf << kIndentString; } From ac2e418a5b1e893c0f2179e2cbae95cca998194b Mon Sep 17 00:00:00 2001 From: Javier Lopez-Gomez Date: Fri, 5 Nov 2021 19:50:13 +0100 Subject: [PATCH 2/3] [tcling] Add test against issue #8622 --- core/metacling/test/TClingCallFuncTests.cxx | 47 +++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/core/metacling/test/TClingCallFuncTests.cxx b/core/metacling/test/TClingCallFuncTests.cxx index d813cb1d0b413..e2a2e1139534d 100644 --- a/core/metacling/test/TClingCallFuncTests.cxx +++ b/core/metacling/test/TClingCallFuncTests.cxx @@ -1,3 +1,4 @@ +#include "TError.h" #include "TInterpreter.h" #include "gtest/gtest.h" @@ -14,6 +15,19 @@ // system, feel free to delete them as these tests here don't represent things the user // should do in his code. +class FilterDiagsRAII { + ErrorHandlerFunc_t fPrevHandler; +public: + FilterDiagsRAII(ErrorHandlerFunc_t fn) : fPrevHandler(::GetErrorHandler()) { + ::SetErrorHandler(fn); + gInterpreter->ReportDiagnosticsToErrorHandler(); + } + ~FilterDiagsRAII() { + gInterpreter->ReportDiagnosticsToErrorHandler(/*enable=*/false); + ::SetErrorHandler(fPrevHandler); + } +}; + TEST(TClingCallFunc, FunctionWrapper) { gInterpreter->Declare(R"cpp( @@ -259,3 +273,36 @@ TEST(TClingCallFunc, DISABLED_OverloadedTemplate) } )cpp"); } + +TEST(TClingCallFunc, FunctionWrapperNodiscard) +{ + gInterpreter->Declare(R"cpp( + struct TClingCallFunc_Nodiscard1 { + #if __cplusplus >= 201703L + [[nodiscard]] + #endif + bool foo(int) { return false; } + }; + )cpp"); + + ClassInfo_t *FooNamespace = gInterpreter->ClassInfo_Factory("TClingCallFunc_Nodiscard1"); + CallFunc_t *mc = gInterpreter->CallFunc_Factory(); + Longptr_t offset = 0; + + gInterpreter->CallFunc_SetFuncProto(mc, FooNamespace, "foo", "int", &offset); + std::string wrapper = gInterpreter->CallFunc_GetWrapperCode(mc); + + { + using ::testing::Not; + using ::testing::HasSubstr; + FilterDiagsRAII RAII([] (int /*level*/, Bool_t /*abort*/, + const char * /*location*/, const char *msg) { + EXPECT_THAT(msg, Not(HasSubstr("-Wunused-result"))); + }); + ASSERT_TRUE(gInterpreter->Declare(wrapper.c_str())); + } + + // Cleanup + gInterpreter->CallFunc_Delete(mc); + gInterpreter->ClassInfo_Delete(FooNamespace); +} From f216f65873d4e5791019be4b7dc266d12e8a5ba6 Mon Sep 17 00:00:00 2001 From: Javier Lopez-Gomez Date: Sun, 7 Nov 2021 19:15:41 +0100 Subject: [PATCH 3/3] [test] Moved `FilterDiagsRAII` class to ROOTUnitTestSupport.h --- core/metacling/test/TClingCallFuncTests.cxx | 15 +----------- .../ROOTUnitTestSupport.h | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/core/metacling/test/TClingCallFuncTests.cxx b/core/metacling/test/TClingCallFuncTests.cxx index e2a2e1139534d..7e1f5c3fbd26a 100644 --- a/core/metacling/test/TClingCallFuncTests.cxx +++ b/core/metacling/test/TClingCallFuncTests.cxx @@ -1,4 +1,4 @@ -#include "TError.h" +#include "ROOTUnitTestSupport.h" #include "TInterpreter.h" #include "gtest/gtest.h" @@ -15,19 +15,6 @@ // system, feel free to delete them as these tests here don't represent things the user // should do in his code. -class FilterDiagsRAII { - ErrorHandlerFunc_t fPrevHandler; -public: - FilterDiagsRAII(ErrorHandlerFunc_t fn) : fPrevHandler(::GetErrorHandler()) { - ::SetErrorHandler(fn); - gInterpreter->ReportDiagnosticsToErrorHandler(); - } - ~FilterDiagsRAII() { - gInterpreter->ReportDiagnosticsToErrorHandler(/*enable=*/false); - ::SetErrorHandler(fPrevHandler); - } -}; - TEST(TClingCallFunc, FunctionWrapper) { gInterpreter->Declare(R"cpp( diff --git a/test/unit_testing_support/ROOTUnitTestSupport.h b/test/unit_testing_support/ROOTUnitTestSupport.h index 2614e00857526..9633567242765 100644 --- a/test/unit_testing_support/ROOTUnitTestSupport.h +++ b/test/unit_testing_support/ROOTUnitTestSupport.h @@ -18,6 +18,9 @@ #ifndef ROOT_UNITTESTSUPPORT_H #define ROOT_UNITTESTSUPPORT_H +#include "TError.h" +#include "TInterpreter.h" + #include "gtest/gtest.h" #include "gmock/gmock.h" @@ -27,6 +30,27 @@ using testing::StrEq; using testing::internal::GetCapturedStderr; using testing::internal::CaptureStderr; using testing::internal::RE; + +/// \brief Allows a user function to filter ROOT/cling diagnostics, e.g. +/// ```c++ +/// FilterDiagsRAII RAII([] (int level, Bool_t abort, +/// const char *location, const char *msg) { +/// EXPECT_THAT(msg, Not(HasSubstr("-Wunused-result"))); +/// }); +/// ``` +class FilterDiagsRAII { + ErrorHandlerFunc_t fPrevHandler; +public: + FilterDiagsRAII(ErrorHandlerFunc_t fn) : fPrevHandler(::GetErrorHandler()) { + ::SetErrorHandler(fn); + gInterpreter->ReportDiagnosticsToErrorHandler(); + } + ~FilterDiagsRAII() { + gInterpreter->ReportDiagnosticsToErrorHandler(/*enable=*/false); + ::SetErrorHandler(fPrevHandler); + } +}; + class ExpectedDiagRAII { public: enum ExpectedDiagKind {