Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot print function pointer #68

Open
evandrocoan opened this issue Jan 6, 2020 · 13 comments · May be fixed by #73
Open

Cannot print function pointer #68

evandrocoan opened this issue Jan 6, 2020 · 13 comments · May be fixed by #73

Comments

@evandrocoan
Copy link
Contributor

evandrocoan commented Jan 6, 2020

#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
Copy link
Contributor Author

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

@c42f
Copy link
Owner

c42f commented Jan 7, 2020

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.

@evandrocoan
Copy link
Contributor Author

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/

@c42f
Copy link
Owner

c42f commented Jan 8, 2020

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.

@evandrocoan
Copy link
Contributor Author

evandrocoan commented Jan 8, 2020

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
Copy link
Contributor Author

@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.

@c42f
Copy link
Owner

c42f commented Jan 14, 2020

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.

@lmachucab
Copy link

lmachucab commented Jan 14, 2020

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.

@c42f
Copy link
Owner

c42f commented Jan 15, 2020

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; };

@lmachucab
Copy link

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.

@evandrocoan
Copy link
Contributor Author

evandrocoan commented Jan 29, 2020

```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.

@c42f
Copy link
Owner

c42f commented Jan 30, 2020

@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 #68 (comment) and my first comment #68 (comment))

@evandrocoan
Copy link
Contributor Author

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 evandrocoan linked a pull request Feb 1, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants