Incorporate PR feedback.

Keep previous implementation when threads are not available.

src/catch2/internal/catch_compiler_capabilities.hpp

@@ -37,10 +37,6 @@
 #    define CATCH_CPP17_OR_GREATER
 #  endif
 
-#  if (__cplusplus >= 202002L) || (defined(_MSVC_LANG) && _MSVC_LANG >= 202002L)
-#    define CATCH_CPP20_OR_GREATER
-#  endif
-
 #endif
 
 // Only GCC compiler should be used in this block, so other compilers trying to

src/catch2/internal/catch_output_redirect.cpp

@@ -5,16 +5,16 @@
 //        https://www.boost.org/LICENSE_1_0.txt)
 
 // SPDX-License-Identifier: BSL-1.0
-#include <catch2/internal/catch_output_redirect.hpp>
 #include <catch2/internal/catch_enforce.hpp>
+#include <catch2/internal/catch_move_and_forward.hpp>
+#include <catch2/internal/catch_output_redirect.hpp>
 #include <catch2/internal/catch_stdstreams.hpp>
-
 #include <cstdio>
 #include <cstring>
 #include <sstream>
 
 #if defined( CATCH_CONFIG_NEW_CAPTURE )
-    #include <catch2/internal/catch_move_and_forward.hpp>
+#    if defined( CATCH_INTERNAL_CONFIG_USE_ASYNC )
 #        include <system_error>
 #        if defined( _MSC_VER )
 #            include <fcntl.h> // _O_TEXT
@@ -26,16 +26,25 @@
 #        else
 #            include <unistd.h> // close, dup, dup2, fileno, pipe and read
 #        endif
+#    else
+#        if defined( _MSC_VER )
+#            include <io.h> //_dup and _dup2
+#            define dup _dup
+#            define dup2 _dup2
+#            define fileno _fileno
+#        else
+#            include <unistd.h> // dup and dup2
+#        endif
+#    endif
 #endif
-
 
 namespace Catch {
 
-    RedirectedStream::RedirectedStream( std::ostream& originalStream, std::ostream& redirectionStream )
+    RedirectedStream::RedirectedStream( std::ostream& originalStream,
-    :   m_originalStream( originalStream ),
+                                        std::ostream& redirectionStream ):
+        m_originalStream( originalStream ),
         m_redirectionStream( redirectionStream ),
-        m_prevBuf( m_originalStream.rdbuf() )
+        m_prevBuf( m_originalStream.rdbuf() ) {
-    {
         m_originalStream.rdbuf( m_redirectionStream.rdbuf() );
     }
 
@@ -43,19 +52,19 @@ namespace Catch {
         m_originalStream.rdbuf( m_prevBuf );
     }
 
-    RedirectedStdOut::RedirectedStdOut() : m_cout( Catch::cout(), m_rss.get() ) {}
+    RedirectedStdOut::RedirectedStdOut():
+        m_cout( Catch::cout(), m_rss.get() ) {}
     auto RedirectedStdOut::str() const -> std::string { return m_rss.str(); }
 
-    RedirectedStdErr::RedirectedStdErr()
+    RedirectedStdErr::RedirectedStdErr():
-    :   m_cerr( Catch::cerr(), m_rss.get() ),
+        m_cerr( Catch::cerr(), m_rss.get() ),
-        m_clog( Catch::clog(), m_rss.get() )
+        m_clog( Catch::clog(), m_rss.get() ) {}
-    {}
     auto RedirectedStdErr::str() const -> std::string { return m_rss.str(); }
 
-    RedirectedStreams::RedirectedStreams(std::string& redirectedCout, std::string& redirectedCerr)
+    RedirectedStreams::RedirectedStreams( std::string& redirectedCout,
-    :   m_redirectedCout(redirectedCout),
+                                          std::string& redirectedCerr ):
-        m_redirectedCerr(redirectedCerr)
+        m_redirectedCout( redirectedCout ),
-    {}
+        m_redirectedCerr( redirectedCerr ) {}
 
     RedirectedStreams::~RedirectedStreams() {
         m_redirectedCout += m_redirectedStdOut.str();
@@ -64,52 +73,46 @@ namespace Catch {
 
 #if defined( CATCH_CONFIG_NEW_CAPTURE )
 
-inline void close_or_throw(int descriptor)
+#    if defined( CATCH_INTERNAL_CONFIG_USE_ASYNC )
-{
+
-    if (close(descriptor))
+    static inline void close_or_throw( int descriptor ) {
-    {
+        if ( close( descriptor ) ) {
             CATCH_SYSTEM_ERROR( errno, std::generic_category() );
         }
     }
 
-inline int dup_or_throw(int descriptor)
+    static inline int dup_or_throw( int descriptor ) {
-{
         int result{ dup( descriptor ) };
 
-    if (result == -1)
+        if ( result == -1 ) {
-    {
             CATCH_SYSTEM_ERROR( errno, std::generic_category() );
         }
 
         return result;
     }
 
-inline int dup2_or_throw(int sourceDescriptor, int destinationDescriptor)
+    static inline int dup2_or_throw( int sourceDescriptor,
-{
+                                     int destinationDescriptor ) {
         int result{ dup2( sourceDescriptor, destinationDescriptor ) };
 
-    if (result == -1)
+        if ( result == -1 ) {
-    {
             CATCH_SYSTEM_ERROR( errno, std::generic_category() );
         }
 
         return result;
     }
 
-inline int fileno_or_throw(std::FILE* file)
+    static inline int fileno_or_throw( std::FILE* file ) {
-{
         int result{ fileno( file ) };
 
-    if (result == -1)
+        if ( result == -1 ) {
-    {
             CATCH_SYSTEM_ERROR( errno, std::generic_category() );
         }
 
         return result;
     }
 
-inline void pipe_or_throw(int descriptors[2])
+    static inline void pipe_or_throw( int descriptors[2] ) {
-{
 #        if defined( _MSC_VER )
         constexpr int defaultPipeSize{ 0 };
 
@@ -118,90 +121,64 @@ inline void pipe_or_throw(int descriptors[2])
         int result{ pipe( descriptors ) };
 #        endif
 
-    if (result)
+        if ( result ) {
-    {
             CATCH_SYSTEM_ERROR( errno, std::generic_category() );
         }
     }
 
-inline size_t read_or_throw(int descriptor, void* buffer, size_t size)
+    static inline size_t
-{
+    read_or_throw( int descriptor, void* buffer, size_t size ) {
 #        if defined( _MSC_VER )
-    int result{ _read(descriptor, buffer, static_cast<unsigned>(size)) };
+        int result{
+            _read( descriptor, buffer, static_cast<unsigned>( size ) ) };
 #        else
         ssize_t result{ read( descriptor, buffer, size ) };
 #        endif
 
-    if (result == -1)
+        if ( result == -1 ) {
-    {
             CATCH_SYSTEM_ERROR( errno, std::generic_category() );
         }
 
         return static_cast<size_t>( result );
     }
 
-inline void fflush_or_throw(std::FILE* file)
+    static inline void fflush_or_throw( std::FILE* file ) {
-{
+        if ( std::fflush( file ) ) {
-    if (std::fflush(file))
-    {
             CATCH_SYSTEM_ERROR( errno, std::generic_category() );
         }
     }
 
-jthread::jthread() noexcept : m_thread{} {}
+    constexpr UniqueFileDescriptor::UniqueFileDescriptor() noexcept:
+        m_value{} {}
 
-template <typename F, typename... Args>
+    UniqueFileDescriptor::UniqueFileDescriptor( int value ) noexcept:
-jthread::jthread(F&& f, Args&&... args) : m_thread{ CATCH_FORWARD(f), CATCH_FORWARD(args)... } {}
+        m_value{ value } {}
 
-// Not exactly like std::jthread, but close enough for the code below.
+    constexpr UniqueFileDescriptor::UniqueFileDescriptor(
-jthread::~jthread() noexcept
+        UniqueFileDescriptor&& other ) noexcept:
-{
+        m_value{ other.m_value } {
-    if (m_thread.joinable())
-    {
-        m_thread.join();
-    }
-}
-
-constexpr UniqueFileDescriptor::UniqueFileDescriptor() noexcept : m_value{} {}
-
-UniqueFileDescriptor::UniqueFileDescriptor(int value) noexcept : m_value{ value } {}
-
-constexpr UniqueFileDescriptor::UniqueFileDescriptor(UniqueFileDescriptor&& other) noexcept :
-    m_value{ other.m_value }
-{
         other.m_value = 0;
     }
 
-UniqueFileDescriptor::~UniqueFileDescriptor() noexcept
+    UniqueFileDescriptor::~UniqueFileDescriptor() noexcept {
-{
+        if ( m_value == 0 ) {
-    if (m_value == 0)
-    {
             return;
         }
 
-    close_or_throw(m_value); // std::terminate on failure (due to noexcept)
+        close_or_throw(
-}
+            m_value ); // std::terminate on failure (due to noexcept)
-
-UniqueFileDescriptor& UniqueFileDescriptor::operator=(UniqueFileDescriptor&& other) noexcept
-{
-    if (this != &other)
-    {
-        if (m_value != 0)
-        {
-            close_or_throw(m_value); // std::terminate on failure (due to noexcept)
-        }
-
-        m_value = other.m_value;
-        other.m_value = 0;
     }
 
+    UniqueFileDescriptor&
+    UniqueFileDescriptor::operator=( UniqueFileDescriptor&& other ) noexcept {
+        std::swap( m_value, other.m_value );
         return *this;
     }
 
     constexpr int UniqueFileDescriptor::get() { return m_value; }
 
-inline void create_pipe(UniqueFileDescriptor& readDescriptor, UniqueFileDescriptor& writeDescriptor)
+    static inline void create_pipe( UniqueFileDescriptor& readDescriptor,
-{
+                                    UniqueFileDescriptor& writeDescriptor ) {
         readDescriptor = {};
         writeDescriptor = {};
 
@@ -212,47 +189,136 @@ inline void create_pipe(UniqueFileDescriptor& readDescriptor, UniqueFileDescript
         writeDescriptor = UniqueFileDescriptor{ descriptors[1] };
     }
 
-inline void read_thread(UniqueFileDescriptor&& file, std::string& result)
+    static inline void read_thread( UniqueFileDescriptor&& file,
-{
+                                    std::string& result ) {
         std::string buffer{};
         constexpr size_t bufferSize{ 4096 };
         buffer.resize( bufferSize );
         size_t sizeRead{};
 
-    while ((sizeRead = read_or_throw(file.get(), &buffer[0], bufferSize)) != 0)
+        while ( ( sizeRead = read_or_throw(
-    {
+                      file.get(), &buffer[0], bufferSize ) ) != 0 ) {
             result.append( buffer.data(), sizeRead );
         }
     }
 
-OutputFileRedirector::OutputFileRedirector(FILE* file, std::string& result) :
+    OutputFileRedirector::OutputFileRedirector( FILE* file,
+                                                std::string& result ):
         m_file{ file },
         m_fd{ fileno_or_throw( m_file ) },
-    m_previous{ dup_or_throw(m_fd) }
+        m_previous{ dup_or_throw( m_fd ) } {
-{
         fflush_or_throw( m_file );
 
         UniqueFileDescriptor readDescriptor{};
         UniqueFileDescriptor writeDescriptor{};
         create_pipe( readDescriptor, writeDescriptor );
 
-    // Anonymous pipes have a limited buffer and require an active reader to ensure the writer does not become blocked.
+        // Anonymous pipes have a limited buffer and require an active reader to
-    // Use a separate thread to ensure the buffer does not get stuck full.
+        // ensure the writer does not become blocked. Use a separate thread to
-    m_readThread = jthread{ [readDescriptor{ CATCH_MOVE(readDescriptor) }, &result] () mutable {
+        // ensure the buffer does not get stuck full.
-        read_thread(CATCH_MOVE(readDescriptor), result); } };
+        m_readThread =
+            std::thread{ [readDescriptor{ CATCH_MOVE( readDescriptor ) },
+                          &result]() mutable {
+                read_thread( CATCH_MOVE( readDescriptor ), result );
+            } };
 
+        // Replace the stdout or stderr file descriptor with the write end of
+        // the pipe.
         dup2_or_throw( writeDescriptor.get(), m_fd );
     }
 
-OutputFileRedirector::~OutputFileRedirector() noexcept
+    OutputFileRedirector::~OutputFileRedirector() noexcept {
-{
+        if ( m_readThread.joinable() ) {
-    fflush_or_throw(m_file); // std::terminate on failure (due to noexcept)
+            m_readThread.join();
-    dup2_or_throw(m_previous.get(), m_fd); // std::terminate on failure (due to noexcept)
+        }
+
+        fflush_or_throw(
+            m_file ); // std::terminate on failure (due to noexcept)
+
+        // Restore the original stdout or stderr file descriptor.
+        dup2_or_throw( m_previous.get(),
+                       m_fd ); // std::terminate on failure (due to noexcept)
     }
 
     OutputRedirect::OutputRedirect( std::string& output, std::string& error ):
         m_output{ stdout, output }, m_error{ stderr, error } {}
 
+#    else // !CATCH_INTERNAL_CONFIG_USE_ASYNC
+
+#        if defined( _MSC_VER )
+    TempFile::TempFile() {
+        if ( tmpnam_s( m_buffer ) ) {
+            CATCH_RUNTIME_ERROR( "Could not get a temp filename" );
+        }
+        if ( fopen_s( &m_file, m_buffer, "w+" ) ) {
+            char buffer[100];
+            if ( strerror_s( buffer, errno ) ) {
+                CATCH_RUNTIME_ERROR( "Could not translate errno to a string" );
+            }
+            CATCH_RUNTIME_ERROR( "Could not open the temp file: '"
+                                 << m_buffer << "' because: " << buffer );
+        }
+    }
+#        else
+    TempFile::TempFile() {
+        m_file = std::tmpfile();
+        if ( !m_file ) {
+            CATCH_RUNTIME_ERROR( "Could not create a temp file." );
+        }
+    }
+
+#        endif
+
+    TempFile::~TempFile() {
+        // TBD: What to do about errors here?
+        std::fclose( m_file );
+        // We manually create the file on Windows only, on Linux
+        // it will be autodeleted
+#        if defined( _MSC_VER )
+        std::remove( m_buffer );
+#        endif
+    }
+
+    FILE* TempFile::getFile() { return m_file; }
+
+    std::string TempFile::getContents() {
+        std::stringstream sstr;
+        char buffer[100] = {};
+        std::rewind( m_file );
+        while ( std::fgets( buffer, sizeof( buffer ), m_file ) ) {
+            sstr << buffer;
+        }
+        return sstr.str();
+    }
+
+    OutputRedirect::OutputRedirect( std::string& stdout_dest,
+                                    std::string& stderr_dest ):
+        m_originalStdout( dup( 1 ) ),
+        m_originalStderr( dup( 2 ) ),
+        m_stdoutDest( stdout_dest ),
+        m_stderrDest( stderr_dest ) {
+        dup2( fileno( m_stdoutFile.getFile() ), 1 );
+        dup2( fileno( m_stderrFile.getFile() ), 2 );
+    }
+
+    OutputRedirect::~OutputRedirect() {
+        Catch::cout() << std::flush;
+        fflush( stdout );
+        // Since we support overriding these streams, we flush cerr
+        // even though std::cerr is unbuffered
+        Catch::cerr() << std::flush;
+        Catch::clog() << std::flush;
+        fflush( stderr );
+
+        dup2( m_originalStdout, 1 );
+        dup2( m_originalStderr, 2 );
+
+        m_stdoutDest += m_stdoutFile.getContents();
+        m_stderrDest += m_stderrFile.getContents();
+    }
+
+#    endif // CATCH_INTERNAL_CONFIG_USE_ASYNC
+
 #endif // CATCH_CONFIG_NEW_CAPTURE
 
 } // namespace Catch

src/catch2/internal/catch_output_redirect.hpp

@@ -8,17 +8,18 @@
 #ifndef CATCH_OUTPUT_REDIRECT_HPP_INCLUDED
 #define CATCH_OUTPUT_REDIRECT_HPP_INCLUDED
 
+#include <catch2/internal/catch_compiler_capabilities.hpp>
 #include <catch2/internal/catch_platform.hpp>
 #include <catch2/internal/catch_reusable_string_stream.hpp>
-#include <catch2/internal/catch_compiler_capabilities.hpp>
-
 #include <cstdio>
 #include <iosfwd>
 #include <string>
 
 #if defined( CATCH_CONFIG_NEW_CAPTURE )
+#    if defined( CATCH_INTERNAL_CONFIG_USE_ASYNC )
 #        include <thread>
 #    endif
+#endif
 
 namespace Catch {
 
@@ -28,13 +29,15 @@ namespace Catch {
         std::streambuf* m_prevBuf;
 
     public:
-        RedirectedStream( std::ostream& originalStream, std::ostream& redirectionStream );
+        RedirectedStream( std::ostream& originalStream,
+                          std::ostream& redirectionStream );
         ~RedirectedStream();
     };
 
     class RedirectedStdOut {
         ReusableStringStream m_rss;
         RedirectedStream m_cout;
+
     public:
         RedirectedStdOut();
         auto str() const -> std::string;
@@ -47,6 +50,7 @@ namespace Catch {
         ReusableStringStream m_rss;
         RedirectedStream m_cerr;
         RedirectedStream m_clog;
+
     public:
         RedirectedStdErr();
         auto str() const -> std::string;
@@ -59,8 +63,10 @@ namespace Catch {
         RedirectedStreams( RedirectedStreams&& ) = delete;
         RedirectedStreams& operator=( RedirectedStreams&& ) = delete;
 
-        RedirectedStreams(std::string& redirectedCout, std::string& redirectedCerr);
+        RedirectedStreams( std::string& redirectedCout,
+                           std::string& redirectedCerr );
         ~RedirectedStreams();
+
     private:
         std::string& m_redirectedCout;
         std::string& m_redirectedCerr;
@@ -70,35 +76,9 @@ namespace Catch {
 
 #if defined( CATCH_CONFIG_NEW_CAPTURE )
 
-#if defined(CATCH_CPP20_OR_GREATER)
+#    if defined( CATCH_INTERNAL_CONFIG_USE_ASYNC )
-using jthread = std::jthread;
-#else
-// Just enough of std::jthread from C++20 for the code below.
-struct jthread final
-{
-    jthread() noexcept;
 
-    template <typename F, typename... Args>
+    struct UniqueFileDescriptor final {
-    jthread(F&& f, Args&&... args);
-
-    jthread(jthread const&) = delete;
-    jthread(jthread&&) noexcept = default;
-
-    jthread& operator=(jthread const&) noexcept = delete;
-
-    // Not exactly like std::jthread, but close enough for the code below.
-    jthread& operator=(jthread&&) noexcept = default;
-
-    // Not exactly like std::jthread, but close enough for the code below.
-    ~jthread() noexcept;
-
-private:
-    std::thread m_thread;
-};
-#endif
-
-struct UniqueFileDescriptor final
-{
         constexpr UniqueFileDescriptor() noexcept;
         explicit UniqueFileDescriptor( int value ) noexcept;
 
@@ -108,7 +88,8 @@ struct UniqueFileDescriptor final
         ~UniqueFileDescriptor() noexcept;
 
         UniqueFileDescriptor& operator=( UniqueFileDescriptor const& ) = delete;
-    UniqueFileDescriptor& operator=(UniqueFileDescriptor&& other) noexcept;
+        UniqueFileDescriptor&
+        operator=( UniqueFileDescriptor&& other ) noexcept;
 
         constexpr int get();
 
@@ -116,8 +97,7 @@ private:
         int m_value;
     };
 
-struct OutputFileRedirector final
+    struct OutputFileRedirector final {
-{
         explicit OutputFileRedirector( std::FILE* file, std::string& result );
 
         OutputFileRedirector( OutputFileRedirector const& ) = delete;
@@ -132,11 +112,10 @@ private:
         std::FILE* m_file;
         int m_fd;
         UniqueFileDescriptor m_previous;
-    jthread m_readThread;
+        std::thread m_readThread;
     };
 
-struct OutputRedirect final
+    struct OutputRedirect final {
-{
         OutputRedirect( std::string& output, std::string& error );
 
     private:
@@ -144,7 +123,53 @@ private:
         OutputFileRedirector m_error;
     };
 
+#    else // !CATCH_INTERNAL_CONFIG_USE_ASYNC
+
+    // Windows's implementation of std::tmpfile is terrible (it tries
+    // to create a file inside system folder, thus requiring elevated
+    // privileges for the binary), so we have to use tmpnam(_s) and
+    // create the file ourselves there.
+    class TempFile {
+    public:
+        TempFile( TempFile const& ) = delete;
+        TempFile& operator=( TempFile const& ) = delete;
+        TempFile( TempFile&& ) = delete;
+        TempFile& operator=( TempFile&& ) = delete;
+
+        TempFile();
+        ~TempFile();
+
+        std::FILE* getFile();
+        std::string getContents();
+
+    private:
+        std::FILE* m_file = nullptr;
+#        if defined( _MSC_VER )
+        char m_buffer[L_tmpnam] = { 0 };
 #        endif
+    };
+
+    class OutputRedirect {
+    public:
+        OutputRedirect( OutputRedirect const& ) = delete;
+        OutputRedirect& operator=( OutputRedirect const& ) = delete;
+        OutputRedirect( OutputRedirect&& ) = delete;
+        OutputRedirect& operator=( OutputRedirect&& ) = delete;
+
+        OutputRedirect( std::string& stdout_dest, std::string& stderr_dest );
+        ~OutputRedirect();
+
+    private:
+        int m_originalStdout = -1;
+        int m_originalStderr = -1;
+        TempFile m_stdoutFile;
+        TempFile m_stderrFile;
+        std::string& m_stdoutDest;
+        std::string& m_stderrDest;
+    };
+
+#    endif // CATCH_INTERNAL_CONFIG_USE_ASYNC
+#endif     // CATCH_CONFIG_NEW_CAPTURE
 
 } // end namespace Catch