c42f / tinyformat

Minimal, type safe printf replacement library for C++
537 stars 75 forks source link

Cpp98 new macros #66

Closed evandrocoan closed 4 years ago

evandrocoan commented 4 years ago

Following from the question: How to easily create fully “variadic” functions with C++ 98 standard?, this pull request improves the C++ 98 "variadic" templates support by a simple tweak on the "variadic" templates macro implementation.

With these new tweaks, instead of writing "variadic" template macros by creating two function definitions:

inline void printfln(const char* fmt)
{
    format(std::cout, fmt);
    std::cout << '\n';
}

#define TINYFORMAT_MAKE_FORMAT_FUNCS(n)                \
                                                       \
template<TINYFORMAT_ARGTYPES(n)>                       \
void printfln(const char* fmt, TINYFORMAT_VARARGS(n))  \
{                                                      \
    format(std::cout, fmt, TINYFORMAT_PASSARGS(n));    \
    std::cout << '\n';                                 \
}

TINYFORMAT_FOREACH_ARGNUM(TINYFORMAT_MAKE_FORMAT_FUNCS)

We can do it creating only a single function definition, removing duplicated code:

#define TINYFORMAT_MAKE_FORMAT_FUNCS(n)               \
                                                      \
TINYFORMAT_ARGTYPES(n,template<,>,)                   \
inline void printfln(                                 \
        const char* fmt TINYFORMAT_VARARGS(n,,))      \
{                                                     \
    format(std::cout, fmt TINYFORMAT_PASSARGS(n,,));  \
    std::cout << '\n';                                \
}

TINYFORMAT_FOREACH_ARGNUM(TINYFORMAT_MAKE_FORMAT_FUNCS)

Also with this new enhancement, the TINYFORMAT_PASSARGS_TAIL macros are not required anymore because TINYFORMAT_PASSARGS can be used instead.

c42f commented 4 years ago

Thanks for the PR!

I'm a bit torn about whether to do this. On the one hand, it avoids the need for stating the body twice. But on the other hand, I feel that this makes the usage syntax a lot more arcane and difficult to get right. (It's also a breaking change in what has been a very stable interface.)

So on balance I feel like this is not an improvement for the likes of TINYFORMAT_MAKE_FORMAT_FUNCS where body code is simple enough that duplication is a non-issue.

But you obviously have some more cases in mind which might be more complex than the way tinyformat itself uses these functions. Perhaps we could explore those a bit if you could give some example code or links to places you'd like to use these utilities?

By the way: all this macro magic in tinyformat is like an extremely simplified version of the boost preprocessor library, cut down to the very minimal basics which are required to do simple expansion of format lists. So another option for you might be to explore whether you could get the same job done using boost if you want to do more general transformations.

evandrocoan commented 4 years ago

I do not see much problem in breaking the interface because an equivalent interface provided. Softwares can break interface. Linux kernel is one which likes to do it on major releases.

Which syntax you did not liked exactly? I did basically two distinct things in two separate commits:

  1. On the first commit I did the minimal to remove the duplicated code. It introduces the syntaxes:
    1. TINYFORMAT_ARGTYPES(n,template<,>,) which could also be used as TINYFORMAT_ARGTYPES(n,,,). I could look into making the three commas optional when not using the template stuff. So you can call it either TINYFORMAT_ARGTYPES(n,template<,>,) or TINYFORMAT_ARGTYPES(n) (as you already did).
    2. TINYFORMAT_VARARGS(n,,), on this one, you pass the second comma when your macro has some other argument before. And you call it without the second comma TINYFORMAT_VARARGS(n,) when your macro is the beginning. This one specifically does not need to trailing comma, i.e., TINYFORMAT_VARARGS(n), I just added one for synchronize with the other call which has two.
  2. On the second commit I indeed did nasty stuff, just for sake of completeness with C++ ... parameter unpack operator. I did not used it anywhere on this project, neither in any other project (yet at least). I was planing to use it here: https://github.com/c42f/tinyformat/blob/4503a27131123e7d0204035aa8625116130d4ebf/tinyformat.h#L968 But C++ 98 does not support braced initializers list, when it would be useless to expand it, and we are obeyed do a recursion to initialize the array.

So on balance I feel like this is not an improvement for the likes of TINYFORMAT_MAKE_FORMAT_FUNCS where body code is simple enough that duplication is a non-issue.

But you obviously have some more cases in mind which might be more complex than the way tinyformat itself uses these functions. Perhaps we could explore those a bit if you could give some example code or links to places you'd like to use these utilities?

I did not had any big case for far. I had duplicated code here: https://github.com/evandrocoan/cppdebugger/commit/93ee20f4324a997d64a4bdc5e190bf6e3fe7eda8, but I managed to remove the code duplication without using this. (I yet did not had done this code at that time, only later I had the insight I could do more tricks with macros to fully support "variadic" templates)

I really do not like duplicating code, even if it is one single like because soon or later I will forget to update it both places. Then, I did this just for the sake of having some definitive way to doing it in the future.

By the way: all this macro magic in tinyformat is like an extremely simplified version of the boost preprocessor library, cut down to the very minimal basics which are required to do simple expansion of format lists.

I never used boost, they have this macro trick with with a better syntax? I still would not use boost, I just would learn how to do it. But would this boost tricks or syntax be portable?

So another option for you might be to explore whether you could get the same job done using boost if you want to do more general transformations.

I do not think that more transformations other than these one can be done. This first commit introduces some basics transformations. The second commit in the other hand, introduces some nasty ones to go over the parenthesis problem as macro arguments.

Anyways, as this is do not add any new functionality to formatting functions, I can release this changes as a header in my other project if you do not like to break your macro syntax.

c42f commented 4 years ago

Linux kernel is one which likes to do it on major releases.

Actually the linux kernel community is very serious about backward compatibility: https://unix.stackexchange.com/questions/235335/why-is-there-a-linux-kernel-policy-to-never-break-user-space

But not to take tinyformat too seriously :-) I'm happy to change APIs here if it's worthwhile. That's what version numbers are for! There's always a certain level of guessing about whether changing the API is "worth it" for some new functionality.

I never used boost, they have this macro trick with with a better syntax? I still would not use boost, I just would learn how to do it. But would this boost tricks or syntax be portable?

Yes the tricks are portable. If you do want to learn about the techniques, the docs are here https://www.boost.org/doc/libs/1_71_0/libs/preprocessor/doc/index.html, and a somewhat relevant example is here: https://stackoverflow.com/questions/44060631/variadic-template-to-define-a-typedef-using-c98. IIRC you can use it to simulate variadic templates (and much more) but I can't remember the details. If you read the implementation you'll see how much complexity is required in a general tool of this kind. It's a lot of complexity which I've tried to reduce to a bare minimum for tinyformat. As a result, tinyformat's implementation is inflexible. I'm open to making it a more flexible, but don't want to stray too far from the mission of "just do printf well".

Regarding my worries about the syntax introduced here, I feel like the following changes in usage make the macros strictly more difficult to read and write:

old:

#   define MAKE_ERROR_FUNC(n)                                            \
    template<TINYFORMAT_ARGTYPES(n)>                                     \
    std::string error(int code, const char* fmt, TINYFORMAT_VARARGS(n))  \
    {                                                                    \
        m_oss.clear();                                                   \
        m_oss << code << ": ";                                           \
        tfm::format(m_oss, fmt, TINYFORMAT_PASSARGS(n));                 \
        return m_oss.str();                                              \

new:

#   define MAKE_ERROR_FUNC(n)                                     \
    template<class T0 TINYFORMAT_ARGTYPES(n,,,,)>                 \
    std::string error(int code,                                   \
            const char* fmt,                                      \
            const T0& v0 TINYFORMAT_VARARGS(n,,))                 \
    {                                                             \
        m_oss.clear();                                            \
        m_oss << code << ": ";                                    \
        tfm::format(m_oss, fmt, v0 TINYFORMAT_PASSARGS(n,,,,));   \
        return m_oss.str();                                       \
    }

Especially the rules for comma placement in the modified version seem quite unintuitive.

Now, I know that you've generalized things and that can be worthwhile. That's why I offered to work through your desired use cases; perhaps we can find some more minimal changes which do what you need and will be more satisfying to everyone.