tinyformat icon indicating copy to clipboard operation
tinyformat copied to clipboard

Cannot print function pointer

Open evandrocoan opened this issue 4 years ago • 13 comments

#include "tinyformat.h"

int main(int argc, char const *argv[])
{
    // typedef int (*Function)( int argc, char const *argv[] );
    std::cerr << tfm::format( "var2 %s", &main ) << std::endl;
    return 0;
}

Results in:

# g++ -o main -g -ggdb test_debugger.cpp --std=c++98 && ./main
var2 true

If I remove the &main and use only main, it does not compile:

# g++ -o main -g -ggdb test_debugger.cpp ---std=c++98 && ./main
In file included from test_debugger.cpp:25:0:
tinyformat.h: In instantiation of ‘static void tinyformat::detail::FormatArg::formatImpl(std::ostream&, const char*, const char*, int, const void*) [with T = int(int, const char**); std::ostream = std::basic_ostream<char>]’:
tinyformat.h:499:38:   required from ‘tinyformat::detail::FormatArg::FormatArg(const T&) [with T = int(int, const char**)]’
tinyformat.h:976:9:   required from ‘void tinyformat::detail::FormatListN<N>::init(int, const T0&) [with T0 = int(int, const char**); int N = 1]’
tinyformat.h:976:9:   required from ‘tinyformat::detail::FormatListN<N>::FormatListN(const T0&) [with T0 = int(int, const char**); int N = 1]’
tinyformat.h:1025:1:   required from ‘tinyformat::detail::FormatListN<1> tinyformat::makeFormatList(const T1&) [with T1 = int(int, const char**)]’
tinyformat.h:1108:1:   required from ‘void tinyformat::format(std::ostream&, const char*, const T1&) [with T1 = int(int, const char**); std::ostream = std::basic_ostream<char>]’
tinyformat.h:1108:1:   required from ‘std::__cxx11::string tinyformat::format(const char*, const T1&) [with T1 = int(int, const char**); std::__cxx11::string = std::__cxx11::basic_string<char>]’
test_debugger.cpp:30:47:   required from here
tinyformat.h:522:57: error: invalid static_cast from type ‘const void*’ to type ‘int (*)(int, const char**)’
             formatValue(out, fmtBegin, fmtEnd, ntrunc, *static_cast<const T*>(value));
                                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
tinyformat.h: In instantiation of ‘static int tinyformat::detail::FormatArg::toIntImpl(const void*) [with T = int(int, const char**)]’:
tinyformat.h:499:38:   required from ‘tinyformat::detail::FormatArg::FormatArg(const T&) [with T = int(int, const char**)]’
tinyformat.h:976:9:   required from ‘void tinyformat::detail::FormatListN<N>::init(int, const T0&) [with T0 = int(int, const char**); int N = 1]’
tinyformat.h:976:9:   required from ‘tinyformat::detail::FormatListN<N>::FormatListN(const T0&) [with T0 = int(int, const char**); int N = 1]’
tinyformat.h:1025:1:   required from ‘tinyformat::detail::FormatListN<1> tinyformat::makeFormatList(const T1&) [with T1 = int(int, const char**)]’
tinyformat.h:1108:1:   required from ‘void tinyformat::format(std::ostream&, const char*, const T1&) [with T1 = int(int, const char**); std::ostream = std::basic_ostream<char>]’
tinyformat.h:1108:1:   required from ‘std::__cxx11::string tinyformat::format(const char*, const T1&) [with T1 = int(int, const char**); std::__cxx11::string = std::__cxx11::basic_string<char>]’
test_debugger.cpp:30:47:   required from here
tinyformat.h:528:45: error: invalid static_cast from type ‘const void*’ to type ‘int (*)(int, const char**)’
             return convertToInt<T>::invoke(*static_cast<const T*>(value));
                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Example using a volatile variable:

#include "tinyformat.h"

int main(int argc, char const *argv[])
{
    typedef int (*Function)( int argc, char const *argv[] );
    volatile Function var1 = reinterpret_cast< volatile Function >( main );

    std::cerr << tfm::format( "volatile var1 %p", var1 ) << std::endl;
    std::cerr << tfm::format( "volatile var1 %s", var1 ) << std::endl;
    return 0;
}

Results in the same

# g++ -o main -g -ggdb test_debugger.cpp --std=c++98 && ./main
volatile var1 1
volatile var1 true

evandrocoan avatar Jan 06 '20 16:01 evandrocoan

Just casting it to a function pointer also does not work:

#include "tinyformat.h"

int main(int argc, char const *argv[])
{
    typedef int (*Function)( int argc, char const *argv[] );
    std::cerr << tfm::format( "var2 %s", (Function) main ) << std::endl;
    return 0;
}

Results:

# g++ -o main.exe --std=c++17 test_debugger.cpp && ./main.exe
var2 true

evandrocoan avatar Jan 06 '20 17:01 evandrocoan

The root cause here is the ambiguity between converting a function pointer to a void* vs to a bool in the underlying std iostreams library. It seems the bool is chosen:

#include <iostream>

int main()
{
    std::cout.setf(std::ios::boolalpha);
    std::cout << &main << "\n";
    std::cout << (void*)&main << "\n";  // The workaround
    return 0;
}

// prints something like
//   true
//   0x55deee04189a

There's already some workarounds for behaving more like printf when using the %p format spec, so if we had the right trait it could probably be dropped in without much disruption. See https://github.com/c42f/tinyformat/blob/master/tinyformat.h#L335

One option may be replacing the is_convertible<T, const void*> trait with something more like is_static_castable, though that's not a standard trait and we'd have to figure out whether it can even be implemented in C++98.

Another option might be to have a special case for pointers with %p format spec, eg using the std::is_pointer trait when deciding whether to cast to const void* before printing... or we could have is_pointer<T> || is_convertible<T, const void*> to take the user's suggestion for %p formatting. I kind of like this latter version of the idea as it's arguably as similar to printf as possible. Just need to implement is_pointer in a C++98 compatible way.

c42f avatar Jan 07 '20 05:01 c42f

I prefer to fix the %s format (in the underlying std iostreams library) because I like to always use %s for everything, including pointers.

If the %s fix is implemented, then, I think it would not be necessary to implement the %p formatting fix, unless we cannot find a way to fix the %s format specifier.

How this is_static_castable should look like? I found this:

  1. https://stackoverflow.com/questions/23762224/check-if-primitive-types-are-castable-in-c
  2. https://searchcode.com/codesearch/view/14603656/

evandrocoan avatar Jan 07 '20 13:01 evandrocoan

I prefer to fix the %s format (in the underlying std iostreams library) because I like to always use %s for everything, including pointers.

I'm not sure this will work out cleanly. Generally the rule for %s is that it will pass through to operator<< without any type conversions so that the user can define their own operator<< for whatever custom types they like. It seems heavy handed to try guessing at what the user meant and converting all pointer types to const void*. A concrete example to consider is the case of const char*. That's a pointer, but is treated as a string if you print it with %s. (%p prints it as a pointer.)

The general point here is that it's part of the public interface that tinyformat uses the standard iostreams operator<< for the task of printing stuff. As a consequence, some of the annoying edge cases of the standard library iostreams are present. That's unfortunate, but I think the alternative of guessing what the user wants may be worse.

How this is_static_castable should look like? I found this:

Unfortunately all those solutions rely on decltype which is not available in C++98. I think it may be better just to use is_pointer which is simple and very easy to implement ourselves, and restrict to improving the %p case.

c42f avatar Jan 08 '20 00:01 c42f

I'm not sure this will work out cleanly.

It would be very nice if we could patch/fix the buggy iostream template overloading resolution for function pointer to bool, i.e.:

  1. Detect if we have a "%s" on the formatting string
  2. Detect if we have a &main a function pointer as the argument
  3. Then only then cast the &main function pointer to (void *) before passing it to the operator<<, fixing the bad iostream template resolution

If these condition are not meet, then we could just do what we usually do, i.e., no conversion before passing it to the operator<<.

Unfortunately all those solutions rely on decltype which is not available in C++98.

I found a decltype implementation for C++ 98:

  1. https://stackoverflow.com/questions/44700418/how-to-implement-decltype-functionality-pre-c11-compile-time
  2. https://stackoverflow.com/questions/12199280/how-to-implement-boost-typeof

This is a minimal working code without requiring the boost library:

#include <iostream>
#include <typeinfo>

template<int N> struct sizer { char value[N]; };

sizer<1> encode(char);
sizer<2> encode(unsigned char);
sizer<3> encode(signed char);
sizer<4> encode(bool);
sizer<5> encode(float);
// ...

template<int N> struct decode {};
template<> struct decode<1> { typedef char type; };
template<> struct decode<2> { typedef unsigned char type; };
template<> struct decode<3> { typedef signed char type; };
template<> struct decode<4> { typedef bool type; };
template<> struct decode<5> { typedef float type; };
// ...

#define TYPEOF(expr) decode<sizeof(encode(expr))>::type

int main() {
    int a; float b;
    TYPEOF(a+b) c = a+b;
    std::cout << typeid(c).name() << std::endl;
}

Compiling it:

$ g++ -o main.exe --std=c++98 test_debugger.cpp && ./main.exe
f

Should it be enough? It's only downside is that all supported types need to be enumerated before hand.

evandrocoan avatar Jan 08 '20 20:01 evandrocoan

@c42f, I found a solution for the iostream overloading problem: https://godbolt.org/z/Hf67j5

#include<iostream>

template<class Ret, class... Args>
std::ostream& operator <<(std::ostream& os, Ret(*p)(Args...) ) {
    return os << "funptr " << (void*)p;
}

template <typename T, typename R, typename ...Args>
std::ostream& operator <<(std::ostream& os, R (T::*p)(Args...) ) {
    return os << "funptr " << (void*)p;
}

struct test_debugger { void var() {} };
void fun_void_void(){};
void fun_void_double(double d){};
double fun_double_double(double d){return d;}

int main() {
    std::cout << "0. " << &test_debugger::var << std::endl;
    std::cout << "1. " << fun_void_void << std::endl;
    std::cout << "2. " << fun_void_double << std::endl;
    std::cout << "3. " << fun_double_double << std::endl;
}

// Prints:
//    0. funptr 0x100401860
//    1. funptr 0x100401080
//    2. funptr 0x100401087
//    3. funptr 0x100401093
  1. https://stackoverflow.com/questions/2064692/how-to-print-function-pointers-with-cout
  2. https://stackoverflow.com/questions/59685972/is-possible-to-fix-the-iostream-cout-cerr-member-function-pointers-being-printed#59686193

We could attach this conversion to some inner layer instead of globally defining it for all iostreams.

evandrocoan avatar Jan 10 '20 17:01 evandrocoan

template<class Ret, class... Args>
std::ostream& operator <<(std::ostream& os, Ret(*p)(Args...) ) {
    return os << "funptr " << (void*)p;
}

Plausibly we could attach some conversion like this as a new version of formatValue and that could be a reasonably neat solution. But note that this still depends on C++11 features.

c42f avatar Jan 14 '20 06:01 c42f

It can be written easily in C++<11, it just translates to N+1 function definitions each taking function pointers to functions with 0, 1, ... N arguments. Also, 2*N if you also need to handle const. Tedious, but it only has to be written once and likely once done won't ever need to change (are there even upgrades to eg.: MSVC 2008 or GCC 4 anymore?). I probably already have it somewhere in my toolkit.

lmachucab avatar Jan 14 '20 13:01 lmachucab

N+1 function definitions

Using a trick with the is_pointer trait (either directly in formatValue or via SFINAE) seems like it would be much shorter and less fragile. Is there a reason that wouldn't work?

Here's a very cut down is_pointer implementation with only value defined (hopefully correct...):

template<typename>   struct is_pointer                    { static const bool value = false; };
template<typename T> struct is_pointer<T*>                { static const bool value = true; };
template<typename T> struct is_pointer<const T*>          { static const bool value = true; };
template<typename T> struct is_pointer<volatile T*>       { static const bool value = true; };
template<typename T> struct is_pointer<const volatile T*> { static const bool value = true; };

c42f avatar Jan 15 '20 00:01 c42f

At a first glance it should work, or at least I can't see a reason why it wouldn't. For maximum compatibility we tag dispatch on the trait and that way we only have to write 2 implementations (so 3 functions counting the dispatcher). I don't know why didn't I think of that before.

lmachucab avatar Jan 15 '20 16:01 lmachucab

```c++
template<class Ret, class... Args>
std::ostream& operator <<(std::ostream& os, Ret(*p)(Args...) ) {
    return os << "funptr " << (void*)p;
}

@c42f Plausibly we could attach some conversion like this as a new version of formatValue and that could be a reasonably neat solution. But note that this still depends on C++11 features.

I just figure out the C++ equivalent for it with C++ 98.

This cannot be done with the current macros because the function pointer signature cannot be something like:

  1. Return(*pointer)( const T0& t0 )
  2. Return(*pointer)( const T0& t0, const T1& t1 )
  3. Return(*pointer)( const T0& t0, const T1& t1, const T2& t2 )
  4. ...

It has to be something like:

  1. Return(*pointer)( T0 t0 )
  2. Return(*pointer)( T0 t0, T1 t1 )
  3. Return(*pointer)( T0 t0, T1 t1, T2 t2 )
  4. ...

Minimal working example: https://godbolt.org/z/PJ66sR

#include<iostream>

template<typename Return>
std::ostream& operator <<(std::ostream& os, Return(*pointer)() ) {
    return os << (void*) pointer;
}

template<typename Return, typename T0>
std::ostream& operator <<(std::ostream& os, Return(*pointer)( T0 t0 ) ) {
    return os << (void*) pointer;
}

template<typename Return, typename T0, typename T1>
std::ostream& operator <<(std::ostream& os, Return(*pointer)( T0 t0, T1 t1 ) ) {
    return os << (void*) pointer;
}

void fun_void_void(){};
void fun_void_double(double d){};
double fun_double_double(double d){return d;}

int main() {
    std::cout << "1. " << fun_void_void << std::endl;
    std::cout << "2. " << fun_void_double << std::endl;
    std::cout << "3. " << fun_double_double << std::endl;
}

// Prints:
//    1. funptr 0x100401080
//    2. funptr 0x100401087
//    3. funptr 0x100401093
  • Even if we are using T0 t0, T1 t1, T2 t2, ..., the function still accepting function pointers with const Type& arg signatures.

evandrocoan avatar Jan 29 '20 22:01 evandrocoan

@evandrocoan I don't think that's the way to go. It relies on enumerating an unbounded list of cases and is a lot of machinery to fix what is — in my view — a small wart.

I think an is_pointer based workaround could be acceptable though (see https://github.com/c42f/tinyformat/issues/68#issuecomment-574443967 and my first comment https://github.com/c42f/tinyformat/issues/68#issuecomment-571438675)

c42f avatar Jan 30 '20 03:01 c42f

For an C++ 98 is_pointer version I found these alternatives: https://stackoverflow.com/questions/3177686/how-to-implement-is-pointer

Which one you like most? This solution I think is nice:

template <typename T> 
struct is_pointer 
{ static const bool value = false; };

template <typename T> 
struct is_pointer<T*> 
{ static const bool value = true; };

But I am not sure if it is the best as the fist answer is quite big:

template <typename T>
struct remove_const
{
    typedef T type;
};

template <typename T>
struct remove_const<const T>
{
    typedef T type;
};

...

evandrocoan avatar Jan 30 '20 13:01 evandrocoan