[MAINT] Added optional test discovery. Added improvements for attributes. Enabled std shared mutex if compiling with C++17 (#3190)

* [MAINT] Added optional test discovery. Added improvements for attributes

* Enabled C++17 version of shared mutex when compiling in C++17 mode

* Added UT discovery option to configure

* Changed enabled std::shared_mutex only if compiling with stdc++-sync

* Fixed problem with not updated sync.cpp SharedMutex

* Fixed review findings

---------

Co-authored-by: Mikolaj Malecki <mmalecki@haivision.com>

CMakeLists.txt

@@ -164,6 +164,7 @@ option(ENABLE_PKTINFO "Enable using IP_PKTINFO to allow the listener extracting
 option(ENABLE_RELATIVE_LIBPATH "Should application contain relative library paths, like ../lib" OFF)
 option(ENABLE_GETNAMEINFO "In-logs sockaddr-to-string should do rev-dns" OFF)
 option(ENABLE_UNITTESTS "Enable unit tests" OFF)
+option(ENABLE_UNITTESTS_DISCOVERY "Do unit test discovery when unit tests enabled" ON)
 option(ENABLE_ENCRYPTION "Enable encryption in SRT" ON)
 option(ENABLE_AEAD_API_PREVIEW "Enable AEAD API preview in SRT" Off)
 option(ENABLE_MAXREXMITBW "Enable SRTO_MAXREXMITBW (v1.6.0 API preview)" Off)
@@ -1585,7 +1586,9 @@ if (ENABLE_UNITTESTS AND ENABLE_CXX11)
 		#set_tests_properties(test-srt PROPERTIES RUN_SERIAL TRUE)
 	else()
 		set_tests_properties(${tests_srt} PROPERTIES RUN_SERIAL TRUE)
-		gtest_discover_tests(test-srt)
+		if (ENABLE_UNITTESTS_DISCOVERY)
+			gtest_discover_tests(test-srt)
+		endif()
 	endif()
 
 	enable_testing()

configure-data.tcl

@@ -49,6 +49,7 @@ set cmake_options {
     enable-relative-libpath "Should applications contain relative library paths, like ../lib (default: OFF)"
     enable-getnameinfo "In-logs sockaddr-to-string should do rev-dns (default: OFF)"
     enable-unittests "Enable Unit Tests (will download Google UT) (default: OFF)"
+    enable-unittests-discovery "Enable UT Discovery (will run when building) (default: ON)"
     enable-encryption "Should encryption features be enabled (default: ON)"
     enable-c++-deps "Extra library dependencies in srt.pc for C language (default: ON)"
     use-static-libstdc++ "Should use static rather than shared libstdc++ (default: OFF)"

srtcore/srt.h

@@ -109,17 +109,21 @@ written by
 
 #define SRT_ATR_DEPRECATED
 #define SRT_ATR_DEPRECATED_PX [[deprecated]]
+#define SRT_ATR_NODISCARD [[nodiscard]]
 
 // GNUG is GNU C/C++; this syntax is also supported by Clang
 #elif defined(__GNUC__)
 #define SRT_ATR_DEPRECATED_PX
 #define SRT_ATR_DEPRECATED __attribute__((deprecated))
+#define SRT_ATR_NODISCARD __attribute__((warn_unused_result))
 #elif defined(_MSC_VER)
 #define SRT_ATR_DEPRECATED_PX __declspec(deprecated)
 #define SRT_ATR_DEPRECATED // no postfix-type modifier
+#define SRT_ATR_NODISCARD _Check_return_
 #else
 #define SRT_ATR_DEPRECATED_PX
 #define SRT_ATR_DEPRECATED
+#define SRT_ATR_NODISCARD
 #endif
 
 #ifdef __cplusplus

srtcore/srt_attr_defs.h

@@ -17,26 +17,17 @@ used by SRT library internally.
 
 // ATTRIBUTES:
 //
-// SRT_ATR_UNUSED: declare an entity ALLOWED to be unused (prevents warnings)
-// ATR_DEPRECATED: declare an entity deprecated (compiler should warn when used)
 // ATR_NOEXCEPT: The true `noexcept` from C++11, or nothing if compiling in pre-C++11 mode
 // ATR_NOTHROW: In C++11: `noexcept`. In pre-C++11: `throw()`. Required for GNU libstdc++.
 // ATR_CONSTEXPR: In C++11: `constexpr`. Otherwise empty.
 // ATR_OVERRIDE: In C++11: `override`. Otherwise empty.
 // ATR_FINAL: In C++11: `final`. Otherwise empty.
 
-#ifdef __GNUG__
-#define ATR_DEPRECATED __attribute__((deprecated))
-#else
-#define ATR_DEPRECATED
-#endif
 
-#if HAVE_CXX11
-#define SRT_ATR_ALIGNAS(n) alignas(n)
-#elif HAVE_GCC
-#define SRT_ATR_ALIGNAS(n) __attribute__((aligned(n)))
+#ifdef __GNUG__
+#define HAVE_GCC 1
 #else
-#define SRT_ATR_ALIGNAS(n)
+#define HAVE_GCC 0
 #endif
 
 #if defined(__cplusplus) && __cplusplus > 199711L
@@ -44,18 +35,15 @@ used by SRT library internally.
 // For gcc 4.7, claim C++11 is supported, as long as experimental C++0x is on,
 // however it's only the "most required C++11 support".
 #if defined(__GXX_EXPERIMENTAL_CXX0X__) && __GNUC__ == 4 && __GNUC_MINOR__ >= 7 // 4.7 only!
-#define ATR_NOEXCEPT
-#define ATR_NOTHROW throw()
-#define ATR_CONSTEXPR
-#define ATR_OVERRIDE
-#define ATR_FINAL
 #else
 #define HAVE_FULL_CXX11 1
-#define ATR_NOEXCEPT noexcept
-#define ATR_NOTHROW noexcept
-#define ATR_CONSTEXPR constexpr
-#define ATR_OVERRIDE override
-#define ATR_FINAL final
+
+#if __cplusplus >= 201703L
+#define HAVE_CXX17 1
+#else
+#define HAVE_CXX17 0
+#endif
+
 #endif
 #elif defined(_MSC_VER) && _MSC_VER >= 1800
 // Microsoft Visual Studio supports C++11, but not fully,
@@ -65,26 +53,41 @@ used by SRT library internally.
 #define HAVE_CXX11 1
 #if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 190023026
 #define HAVE_FULL_CXX11 1
+
+#if __cplusplus >= 201703L
+#define HAVE_CXX17 1
+#else
+#define HAVE_CXX17 0
+#endif
+
+#endif
+#else
+#define HAVE_CXX11 0
+#define HAVE_CXX17 0
+#endif // __cplusplus
+
+#if HAVE_FULL_CXX11
 #define ATR_NOEXCEPT noexcept
 #define ATR_NOTHROW noexcept
 #define ATR_CONSTEXPR constexpr
 #define ATR_OVERRIDE override
 #define ATR_FINAL final
 #else
+// These are both for HAVE_CXX11 == 1 and 0.
 #define ATR_NOEXCEPT
 #define ATR_NOTHROW throw()
 #define ATR_CONSTEXPR
 #define ATR_OVERRIDE
 #define ATR_FINAL
 #endif
+
+#if HAVE_CXX11
+#define SRT_ATR_ALIGNAS(n) alignas(n)
+#elif HAVE_GCC
+#define SRT_ATR_ALIGNAS(n) __attribute__((aligned(n)))
 #else
-#define HAVE_CXX11 0
-#define ATR_NOEXCEPT
-#define ATR_NOTHROW throw()
-#define ATR_CONSTEXPR
-#define ATR_OVERRIDE
-#define ATR_FINAL
-#endif // __cplusplus
+#define SRT_ATR_ALIGNAS(n)
+#endif
 
 #if !HAVE_CXX11 && defined(REQUIRE_CXX11) && REQUIRE_CXX11 == 1
 #error "The currently compiled application required C++11, but your compiler doesn't support it."

srtcore/sync.cpp

@@ -357,6 +357,11 @@ int srt::sync::genRandomInt(int minVal, int maxVal)
 #endif // HAVE_CXX11
 }
 
+#if defined(ENABLE_STDCXX_SYNC) && HAVE_CXX17
+
+// Shared mutex imp not required - aliased from C++17
+
+#else
 
 ////////////////////////////////////////////////////////////////////////////////
 //
@@ -451,4 +456,6 @@ int srt::sync::SharedMutex::getReaderCount() const
 {
     ScopedLock lk(m_Mutex);
     return m_iCountRead;
-}
+}
+#endif // C++17 for shared_mutex
+

srtcore/sync.h

@@ -12,6 +12,7 @@
 #define INC_SRT_SYNC_H
 
 #include "platform_sys.h"
+#include "srt_attr_defs.h"
 
 #include <cstdlib>
 #include <limits>
@@ -21,6 +22,9 @@
 #include <mutex>
 #include <condition_variable>
 #include <atomic>
+#if HAVE_CXX17
+#include <shared_mutex>
+#endif
 #define SRT_SYNC_CLOCK SRT_SYNC_CLOCK_STDCXX_STEADY
 #define SRT_SYNC_CLOCK_STR "STDCXX_STEADY"
 #else
@@ -54,7 +58,6 @@
 
 #include "srt.h"
 #include "utilities.h"
-#include "srt_attr_defs.h"
 
 
 namespace srt
@@ -491,11 +494,15 @@ inline void releaseCond(Condition& cv) { cv.destroy(); }
 //
 ///////////////////////////////////////////////////////////////////////////////
 
+#if defined(ENABLE_STDCXX_SYNC) && HAVE_CXX17
+using SharedMutex = std::shared_mutex;
+#else
+
 /// Implementation of a read-write mutex. 
 /// This allows multiple readers at a time, or a single writer.
 /// TODO: The class can be improved if needed to give writer a preference
 /// by adding additional m_iWritersWaiting member variable (counter).
-/// TODO: The m_iCountRead could be made atomic to make unlok_shared() faster and lock-free.
+/// TODO: The m_iCountRead could be made atomic to make unlock_shared() faster and lock-free.
 class SharedMutex
 {
 public:
@@ -526,6 +533,7 @@ protected:
     int  m_iCountRead;
     bool m_bWriterLocked;
 };
+#endif
 
 /// A version of std::scoped_lock<std::shared_mutex> (or lock_guard for C++11).
 /// We could have used the srt::sync::ScopedLock making it a template-based class.

test/test_sync.cpp

@@ -609,6 +609,8 @@ TEST(SyncThread, Joinable)
     EXPECT_FALSE(foo.joinable());
 }
 
+#if !HAVE_CXX17
+
 /*****************************************************************************/
 /*
  * SharedMutex
@@ -693,6 +695,7 @@ TEST(SharedMutex, LockedReadCount)
     EXPECT_TRUE(mut.try_lock());
 }
 
+#endif
 
 /*****************************************************************************/
 /*