Skip to content

Constexpr type signatures for C++11 and semi-constexpr for MSVC #934

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

Merged
merged 3 commits into from
Sep 16, 2017

Conversation

dean0x7d
Copy link
Member

@dean0x7d dean0x7d commented Jul 3, 2017

This PR completely removes the runtime descr code path and moves everything (on gcc/clang) or almost everything (on MSVC) to constexpr.

The current C++14 constexpr signatures don't require relaxed constexpr, but only auto return type deduction. To get around this in C++11, the type caster's name() static member functions are turned into static constexpr auto variables.

MSCV does not allow &typeid(T) in constexpr contexts, but the string part of the type signature can still be constexpr. In order to avoid typeid as long as possible, descr is modified to collect type information as template parameters instead of constexpr typeid. The actual std::type_info pointers are only collected in the end, as a constexpr (gcc/clang) or regular (MSVC) function call.

Not only does this approach significantly reduce binary size on MSVC, gcc/clang benefit a little bit as well, since they can skip some intermediate std::type_info* arrays.

Binary size changes:

# C++11 and MSVC see big improvements
GCC4:      -13.6%
VS2015/17:  -8.1%

# Very minor improvement in C++14
GCC6/7: -0.8%
clang4: -0.25%

@jagerman
Copy link
Member

jagerman commented Jul 3, 2017

It's a very nice change; the dual C++14/C++11 logic has always seemed a bit wasteful.

Two questions:

  • By making the name field a static member, aren't we providing a definition without declaration, which has the potential to lead to issues like Unedfined symbol pybind11::object::borrowed #770?
  • This is changing the (public) type_caster interface in the case that someone used their own name(). Do you think it's worth dealing with this (for example, by renaming the static name to type_name and providing a SFINAE function that uses type_name with a name() fallback)? Or should we just consider this an internal detail, since the documented approach doesn't mention name() at all, but just the TYPE_CASTER macro?

@dean0x7d
Copy link
Member Author

dean0x7d commented Jul 3, 2017

By making the name field a static member, aren't we providing a definition without declaration, which has the potential to lead to issues like #770?

It's perfectly fine here because static constexpr auto signature = ... is explicitly asking for a compile-time constant. The constexpr part ensures that all the individual type_caster::name variables will be concatenated at compile-time (no definitions needed). A new variable signature will be created for which we do need a definition, and the handy function-scope static ensures the compiler will store it in the binary.

What effectively happened with that previous issue is:

void runtime_foo(py::detail::descr<8, T1, T2> const&); 

runtime_foo(make_caster<T>::name); // ODR-used here => `::name` needs a definition

So, mixed run time and compile time. But with descr, everything is kept strictly in a constexpr context and it's used exclusively by signature which will be correctly stored for runtime access.

This is changing the (public) type_caster interface in the case that someone used their own name(). Do you think it's worth dealing with this (for example, by renaming the static name to type_name and providing a SFINAE function that uses type_name with a name() fallback)? Or should we just consider this an internal detail, since the documented approach doesn't mention name() at all, but just the TYPE_CASTER macro?

Pretty much the last part. Considering it's in the detail namespace and only PYBIND11_TYPE_CASTER is documented, I'd vote for supporting backward compatibility of PYBIND11_TYPE_CASTER (which this does) but nothing beyond that.

Long term, I think we all agree, it would be good to pull type_caster out of the detail namespace, stabilize and fully document it. I have another PR pending to get it moving in this direction. As far as I see, this can all be done while maintaining PYBIND11_TYPE_CASTER, load() and cast() compatible as is currenly documented, but not the internal details.

@jagerman jagerman mentioned this pull request Jul 21, 2017
6 tasks
@jagerman jagerman added this to the v2.3 milestone Jul 23, 2017
@dean0x7d
Copy link
Member Author

dean0x7d commented Sep 1, 2017

I've rebased this and added one more change which simplifies signature generation. Instead of calling type_descr for each type name, it's now applied only as the final step in argument_loader.

For example, where the constexpr signature was previously,

({int}, {Tuple[{float}, {str}, {%}]}, {Callable[[{int}, {bool}], {int}]}) -> {None}

it's now,

({int}, {Tuple[float, str, %]}, {Callable[[int, bool], int]}) -> None

There's only one level of {} args so there's no need to track type_depth when parsing the signature. As far as I can tell, marking arguments with multiple depth levels isn't being used for anything else. The reduction in the number of {} pairs also reduces binary size ever so slighty.

The final signature is still the same:

(arg0: int, arg1: Tuple[float, str, UserType], arg2: Callable[[int, bool], int]) -> None

The current C++14 constexpr signatures don't require relaxed constexpr,
but only `auto` return type deduction. To get around this in C++11,
the type caster's `name()` static member functions are turned into
`static constexpr auto` variables.
MSCV does not allow `&typeid(T)` in constexpr contexts, but the string
part of the type signature can still be constexpr. In order to avoid
`typeid` as long as possible, `descr` is modified to collect type
information as template parameters instead of constexpr `typeid`.
The actual `std::type_info` pointers are only collected in the end,
as a `constexpr` (gcc/clang) or regular (MSVC) function call.

Not only does it significantly reduce binary size on MSVC, gcc/clang
benefit a little bit as well, since they can skip some intermediate
`std::type_info*` arrays.
`type_descr` is now applied only to the final signature so that it only
marks the argument types, but not nested types (e.g. for tuples) or
return types.
@dean0x7d dean0x7d merged commit 0aef642 into pybind:master Sep 16, 2017
@dean0x7d
Copy link
Member Author

Merged for v2.3.

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 this pull request may close these issues.

2 participants