CODEWITHSUNDEEP
c++c++20
Soleil
Soleil

Reputation: 7457

How to specialise the function or convert the first arg of a function using a perfect forward?

I try to merge two functions into a more general function:

template<class ...T>
auto LoggerBase::LogMessage(T&&... args) -> void
{
    if (Verbose)
    {
        constexpr int size{ 256 };
        char msgBuffer[size];
        auto size_copied = sprintf_s(msgBuffer, size, std::forward<T>(args)...);
        std::string msg(msgBuffer);
        LogMessageQuery.Add(msg);
    }
}

template<class ...T>
auto LoggerBase::LogMessageW(T&&... args) -> void
{
    if (Verbose)
    {
        constexpr int size{ 256 };
        wchar_t msgBuffer[size];
        auto size_copied = swprintf_s(msgBuffer, size, std::forward<T>(args)...);
        std::wstring wmsg(msgBuffer);
        LogMessageQuery.Add(wmsg);
    }
}

into:

template<class ...T>
auto LoggerBase::LogMessageAny(T&&... args) -> void
{
    if (Verbose)
    {
        using firstType = typename std::tuple_element<0, std::tuple<T...>>::type;
        constexpr int size{ 256 };
        if (typeid(firstType) == typeid(const wchar_t*))
        {
            wchar_t msgBuffer[size];
            auto size_copied = swprintf_s(msgBuffer, size, std::forward<T>(args)...);
            std::wstring wmsg(msgBuffer);
            LogMessageQuery.Add(wmsg);
        }
        else if (typeid(firstType) == typeid(const char*))
        {
            char msgBuffer[size];
            auto size_copied = sprintf_s(msgBuffer, size, std::forward<T>(args)...);
            std::string msg(msgBuffer);
            LogMessageQuery.Add(msg);
        }
    }
}

however those test won't compile:

typedef void(__stdcall* MessageChangedCallback)(const wchar_t* string);
static MessageChangedCallback log = [](const wchar_t* z)
    {
        Logger::WriteMessage(z);
    };

TEST_CLASS(LoggerBaseTests)
{
public:
    TEST_METHOD(LogMessageAny_stringTest)
    {
        LoggerBase sut(log);
        sut.LogMessageAny("This is a good std::string TEST %i!", 123);
    }

    TEST_METHOD(LogMessageAny_wstringTest)
    {
        LoggerBase sut(log);
        sut.LogMessageAny(L"This is a good std::wstring TEST %i!", 456);
    }

};

Severity Code Description Project File Line Suppression State Details Error C2664 'int swprintf_s(wchar_t *const ,const size_t,const wchar_t *const ,...)': cannot convert argument 3 from 'const char [36]' to 'const wchar_t *const ' UtilTests C:\Users\soleil\source\repos\SublimeTriptych\util\LoggerBase.h 78

Severity Code Description Project File Line Suppression State Details Error C2664 'int sprintf_s(char *const ,const size_t,const char *const ,...)': cannot convert argument 3 from 'const wchar_t [37]' to 'const char *const ' UtilTests C:\Users\soleil\source\repos\SublimeTriptych\util\LoggerBase.h 85

The inital two functions do work fine.

  1. Is my type switch proper ?

  2. How can I specialise the first arg and forward the rest ? ie., at the call of sprintf(): something in the mindset of swprintf_s(msgBuffer, size, (const char*)args[0], std::forward<T>(args[1..])...)

How to specializing a forwarding parameter pack?

do work with two parameters, but I need to keep a generic parameter pack for args of index >1, therefore it's not a solution.

  1. Should I specialise already in the parameters of the function LogMessage() and overload it instead ?

Upvotes: 5

Views: 129

Answers (2)

Remy Lebeau
Remy Lebeau

Reputation: 597906

Since you always need a format string, you should use a separate template argument to deduce its type, and then you can use if constexpr to make decisions on that type at compile-time rather than at run-time, eg:

#include <type_traits>

template<class CharT, class ...T>
void LoggerBase::LogMessageAny(const CharT *fmt, T&&... args)
{
    static_assert(std::is_same_v<CharT, wchar_t> || std::is_same_v<CharT, char>);

    if (Verbose)
    {
        constexpr int size{ 256 };
        if constexpr (std::is_same_v<CharT, wchar_t>)
        {
            wchar_t msgBuffer[size];
            auto size_copied = swprintf_s(msgBuffer, size, fmt, std::forward<T>(args)...);
            std::wstring wmsg(msgBuffer, size_copied);
            LogMessageQuery.Add(wmsg);
        }
        else
        {
            char msgBuffer[size];
            auto size_copied = sprintf_s(msgBuffer, size, fmt, std::forward<T>(args)...);
            std::string msg(msgBuffer, size_copied);
            LogMessageQuery.Add(msg);
        }
    }
}

And then you can simplify the code further by taking advantage of the fact that std::string and std::wstring are just specializations of std::basic_string, eg:

#include <type_traits>

template<class CharT, class ...T>
void LoggerBase::LogMessageAny(const CharT *fmt, T&&... args)
{
    static_assert(std::is_same_v<CharT, wchar_t> || std::is_same_v<CharT, char>);

    if (Verbose)
    {
        constexpr int size{ 256 };
        CharT msgBuffer[size];
        std::basic_string<CharT>::size_type size_copied;

        if constexpr (std::is_same_v<CharT, wchar_t>)
        {
            size_copied = swprintf_s(msgBuffer, size, fmt, std::forward<T>(args)...);
        }
        else
        {
            size_copied = sprintf_s(msgBuffer, size, fmt, std::forward<T>(args)...);
        }

        std::basic_string<CharT> msg(msgBuffer, size_copied);
        LogMessageQuery.Add(msg);
    }
}

And then you can break out the ...printf functions into their own wrappers and get rid of if constexpr, eg:

template<size_t MsgBufferSize, class ...T>
int printf_t(wchar_t (&msgBuffer)[MsgBufferSize], const wchar_t* fmt, T&&... args)
{
    return swprintf_s(msgBuffer, MsgBufferSize, fmt, std::forward<T>(args)...);
}

template<size_t MsgBufferSize, class ...T>
int printf_t(char (&msgBuffer)[MsgBufferSize], const char* fmt, T&&... args)
{
    return sprintf_s(msgBuffer, MsgBufferSize, fmt, std::forward<T>(args)...);
}

template<class CharT, class ...T>
void LoggerBase::LogMessageAny(const CharT *fmt, T&&... args)
{
    if (Verbose)
    {
        CharT msgBuffer[256];
        int size_copied = printf_t(msgBuffer, fmt, std::forward<T>(args)...);
        std::basic_string<CharT> msg(msgBuffer, size_copied);
        LogMessageQuery.Add(msg);
    }
}

Upvotes: 8

Igor G
Igor G

Reputation: 2471

Is my type switch proper ?

No. You've used runtime if statement, which means that both branches are compiled, and hence both branches must be well-formed. Whereas in your example one branch will always be ill-formed: either narrow formatting function called with wide format string, or wide formatting function called with narrow format string.

How can I specialise the first arg and forward the rest ?

You can either leave two overloads:

template<class ...T>
auto LoggerBase::LogMessage(const char* format, T&&... args) -> void;

template<class ...T>
auto LoggerBase::LogMessage(const wchar_t* format, T&&... args) -> void;

Or you can have a single function and use compile-time switch:

if constexpr (std::is_same_v<std::decay_t<firstType>, const char*>)
{
}
if constexpr (std::is_same_v<std::decay_t<firstType>, const wchar_t*>)
{
}
else
{
    // Error
}

The first approach (two overloads) is much more clean and maintainable, IMO.

Also, do keep in mind that the second approach will likely instantiate a lot of useless functions. If client code uses raw string literals in each call to such logger, there will be one instantiation for each length of the format string, because the first argument will never be deduced as const char*, but rather as const char [5], const char [42], etc.

Upvotes: 5

Related Questions