Add `type_util` meta-programming utilities. #7682

EricCousineau-TRI · 2018-01-02T19:03:21Z

This adds some basic meta-programming utilities for handling template parameter packs (and a little bit of sugar for type hashing).

This related #7660: template classes and methods in pybind11.

For a protoype, see Python output for binding basic templates and a Python derived-class scalar conversion example.

NOTE: Compilation of this code seems significantly slower on my desktop than on my laptop. Will check and see if there's anything that can be changed to fix this.

This change is

EricCousineau-TRI · 2018-01-02T21:13:40Z

@drake-jenkins-bot mac-sierra-clang-bazel-experimental please.
@drake-jenkins-bot mac-highsierra-clang-bazel-experimental please.

EricCousineau-TRI · 2018-01-02T21:14:34Z

+@ggould-tri for feature review, please.
+@jwnimmer-tri for platform review, please.

Review status: 0 of 3 files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

EricCousineau-TRI · 2018-01-02T21:23:24Z

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion.

common/BUILD.bazel, line 354 at r1 (raw file):

    hdrs = ["type_util.h"],
    deps = [
        ":nice_type_name",

Will move nice_type_name this to type_util_test.

Comments from Reviewable

ggould-tri · 2018-01-02T21:41:51Z

Kind of amazing!

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

common/type_util.h, line 66 at r1 (raw file):

/// Extracts the Ith type from a sequence of types.

BTW -- is the compiler error message for an overlarge I remotely comprehensible?

common/type_util.h, line 104 at r1 (raw file):

using type_pack_extract = typename detail::type_pack_extract_impl<T>::type;

/// Visit a type by constructing its default value.

BTW -- is the compiler error message for a class without a default ctor remotely comprehensible?

common/type_util.h, line 129 at r1 (raw file):

};

/// Provides backport of C++17 `std::negation`.

FYI -- you should probably TODO removing this when we get to party like it's C++17.

common/test/type_util_test.cc, line 80 at r1 (raw file):

  EXPECT_EQ(names, names_expected);

  // N.B. `Check` must operate on the exact types containd within the pack.

FYI typo "containd"

common/test/type_util_test.cc, line 82 at r1 (raw file):

  // N.B. `Check` must operate on the exact types containd within the pack.
  // If we used `PackTags{}`, then we should use
  // `check_different_from<type_tag<double>>{}`.

BTW -- would it make sense to test that case as well in addition to describing it?

common/test/type_util_test.cc, line 94 at r1 (raw file):

      constant_add<int, 5>{}, std::make_integer_sequence<int, 5>{});
  EXPECT_TRUE((std::is_same<
      decltype(seq), std::integer_sequence<int, 5, 6, 7, 8, 9>>::value));

This is beautiful sorcery and I fear it.

Comments from Reviewable

EricCousineau-TRI · 2018-01-02T22:17:57Z

Thanks!

Review status: 0 of 3 files reviewed at latest revision, 6 unresolved discussions.

common/BUILD.bazel, line 354 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Will move nice_type_name this to type_util_test.

Done.

common/type_util.h, line 66 at r1 (raw file):

Previously, ggould-tri wrote…

BTW -- is the compiler error message for an overlarge I remotely comprehensible?

Done. Yup! (added in check for negative numbers too, just in case size_t is signed).

In file included from common/test/type_util_test.cc:1:
bazel-out/clang-4.0-linux-opt/bin/common/_virtual_includes/type_util/drake/common/type_util.h:69:3: error: static_assert failed "Invalid type index"
  static_assert(I >= 0 && I < sizeof...(Ts), "Invalid type index");
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~
common/test/type_util_test.cc:27:15: note: in instantiation of template class 'drake::type_at<3, int, double, char>' requested here
  using T_2 = type_at<3, int, double, char>::type;
              ^
common/test/type_util_test.cc:28:29: error: use of undeclared identifier 'T_2'
  EXPECT_TRUE((std::is_same<T_2, char>::value));
                            ^
common/test/type_util_test.cc:28:41: error: no type named 'value' in the global namespace
  EXPECT_TRUE((std::is_same<T_2, char>::value));
                                      ~~^
external/gtest/googletest/include/gtest/gtest.h:1860:24: note: expanded from macro 'EXPECT_TRUE'
  GTEST_TEST_BOOLEAN_((condition), #condition, false, true, \
                       ^~~~~~~~~
external/gtest/googletest/include/gtest/internal/gtest-internal.h:1189:34: note: expanded from macro 'GTEST_TEST_BOOLEAN_'
      ::testing::AssertionResult(expression)) \
                                 ^~~~~~~~~~
3 errors generated.

common/type_util.h, line 104 at r1 (raw file):

Previously, ggould-tri wrote…

BTW -- is the compiler error message for a class without a default ctor remotely comprehensible?

Done. It's only slight comprehensible; unfortunately, it doesn't explicitly state with T = ..., so you have to kind of infer it with the lambda junk (e.g. drake::type_visit_impl<...>::run<void, drake::(anonymous names)::(lambda ... ...)).
Below the assertion, it will spit out something about the invalid initialization.

Unfortunately, I don't know how to bubble up T in the error message, as static_assert is fairly strict on what it permits in its message (at least from what I remember playing with it a while back).

common/type_util.h, line 129 at r1 (raw file):

Previously, ggould-tri wrote…

FYI -- you should probably TODO removing this when we get to party like it's C++17.

Done.

common/test/type_util_test.cc, line 80 at r1 (raw file):

Previously, ggould-tri wrote…

FYI typo "containd"

Done.

common/test/type_util_test.cc, line 82 at r1 (raw file):

Previously, ggould-tri wrote…

BTW -- would it make sense to test that case as well in addition to describing it?

Done.

common/test/type_util_test.cc, line 94 at r1 (raw file):

Previously, ggould-tri wrote…

This is beautiful sorcery and I fear it.

Yeah, it's a tad bit odd, but was useful when wanting to expose literal template parameters to pybind11:
https://github.com/EricCousineau-TRI/repro/blob/6659e31cf1d1234ee0f8b51e54bcd30ca264c6b6/python/bindings/pymodule/tpl/cpp_types.cc#L132

Comments from Reviewable

ggould-tri · 2018-01-02T22:23:25Z

Reviewed 2 of 3 files at r2.
Review status: 2 of 3 files reviewed at latest revision, 2 unresolved discussions.

common/type_util.h, line 66 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Done. Yup! (added in check for negative numbers too, just in case size_t is signed).

In file included from common/test/type_util_test.cc:1:
bazel-out/clang-4.0-linux-opt/bin/common/_virtual_includes/type_util/drake/common/type_util.h:69:3: error: static_assert failed "Invalid type index"
  static_assert(I >= 0 && I < sizeof...(Ts), "Invalid type index");
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~
common/test/type_util_test.cc:27:15: note: in instantiation of template class 'drake::type_at<3, int, double, char>' requested here
  using T_2 = type_at<3, int, double, char>::type;
              ^
common/test/type_util_test.cc:28:29: error: use of undeclared identifier 'T_2'
  EXPECT_TRUE((std::is_same<T_2, char>::value));
                            ^
common/test/type_util_test.cc:28:41: error: no type named 'value' in the global namespace
  EXPECT_TRUE((std::is_same<T_2, char>::value));
                                      ~~^
external/gtest/googletest/include/gtest/gtest.h:1860:24: note: expanded from macro 'EXPECT_TRUE'
  GTEST_TEST_BOOLEAN_((condition), #condition, false, true, \
                       ^~~~~~~~~
external/gtest/googletest/include/gtest/internal/gtest-internal.h:1189:34: note: expanded from macro 'GTEST_TEST_BOOLEAN_'
      ::testing::AssertionResult(expression)) \
                                 ^~~~~~~~~~
3 errors generated.

That's surprisingly readable. Yay.

common/type_util.h, line 104 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Done. It's only slight comprehensible; unfortunately, it doesn't explicitly state with T = ..., so you have to kind of infer it with the lambda junk (e.g. drake::type_visit_impl<...>::run<void, drake::(anonymous names)::(lambda ... ...)).
Below the assertion, it will spit out something about the invalid initialization.

Unfortunately, I don't know how to bubble up T in the error message, as static_assert is fairly strict on what it permits in its message (at least from what I remember playing with it a while back).

Alas. I'll mark this satisfied, but users may be very sad if they try to use this. With any luck, they won't.

Comments from Reviewable

ggould-tri · 2018-01-03T14:39:53Z

Reviewed 1 of 3 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

jwnimmer-tri · 2018-01-03T14:44:54Z

Checkpoint.

Reviewed 1 of 3 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

common/BUILD.bazel, line 21 at r2 (raw file):

# This should encompass every non-testonly cc_library in this package,
# except for items that should only ever be linked into main() programs.

The new :type_util is missing from the below.

Comments from Reviewable

EricCousineau-TRI · 2018-01-03T15:04:57Z

Review status: 1 of 3 files reviewed at latest revision, 1 unresolved discussion.

common/BUILD.bazel, line 21 at r2 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

The new :type_util is missing from the below.

Done.

Comments from Reviewable

ggould-tri · 2018-01-03T15:17:20Z

Reviewed 2 of 2 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

EricCousineau-TRI · 2018-01-03T15:31:42Z

Review status: 1 of 3 files reviewed at latest revision, all discussions resolved.

common/type_util.h, line 104 at r1 (raw file):

Previously, ggould-tri wrote…

Alas. I'll mark this satisfied, but users may be very sad if they try to use this. With any luck, they won't.

BTW I was able to bubble up a little more info by sticking the assertion in a relatively bare structure.
However, the compiler still throws an error down below about T{}.

The errors now look like this, still with a bunch of cruft, but at least calling out drake::detail::assert_default_constructible<void>:

In file included from common/test/type_util_test.cc:1:
bazel-out/clang-4.0-linux-opt/bin/common/_virtual_includes/type_util/drake/common/type_util.h:66:3: error: static_assert failed "Must be default constructible"
  static_assert(std::is_default_constructible<T>::value,
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
bazel-out/clang-4.0-linux-opt/bin/common/_virtual_includes/type_util/drake/common/type_util.h:119:11: note: in instantiation of template class 'drake::detail::assert_default_constructible<void>' requested here
    (void)detail::assert_default_constructible<T>{};
          ^
bazel-out/clang-4.0-linux-opt/bin/common/_virtual_includes/type_util/drake/common/type_util.h:39:27: note: in instantiation of function template specialization 'drake::visit_with_default::run<void, (lambda at co
mmon/test/type_util_test.cc:74:18) &>' requested here
      VisitWith::template run<T>(std::forward<Visitor>(visitor));
                          ^
bazel-out/clang-4.0-linux-opt/bin/common/_virtual_includes/type_util/drake/common/type_util.h:166:15: note: in instantiation of member function 'drake::detail::type_visit_impl<drake::visit_with_default, (lambda 
at common/test/type_util_test.cc:74:18) &>::runner<void, true>::run' requested here
              run(std::forward<Visitor>(visitor)),
              ^
common/test/type_util_test.cc:79:3: note: in instantiation of function template specialization 'drake::type_visit<drake::visit_with_default, drake::check_always_true, (lambda at common/test/type_util_test.cc:74:
18) &, int, double, char, void>' requested here
  type_visit(visitor, Pack{}); //PackTags{});
  ^
In file included from common/test/type_util_test.cc:1:
bazel-out/clang-4.0-linux-opt/bin/common/_virtual_includes/type_util/drake/common/type_util.h:120:14: error: illegal initializer type 'void'
    visitor(T{});
             ^
bazel-out/clang-4.0-linux-opt/bin/common/_virtual_includes/type_util/drake/common/type_util.h:39:27: note: in instantiation of function template specialization 'drake::visit_with_default::run<void, (lambda at co
mmon/test/type_util_test.cc:74:18) &>' requested here
      VisitWith::template run<T>(std::forward<Visitor>(visitor));
                          ^
bazel-out/clang-4.0-linux-opt/bin/common/_virtual_includes/type_util/drake/common/type_util.h:166:15: note: in instantiation of member function 'drake::detail::type_visit_impl<drake::visit_with_default, (lambda 
at common/test/type_util_test.cc:74:18) &>::runner<void, true>::run' requested here
              run(std::forward<Visitor>(visitor)),
              ^
common/test/type_util_test.cc:79:3: note: in instantiation of function template specialization 'drake::type_visit<drake::visit_with_default, drake::check_always_true, (lambda at common/test/type_util_test.cc:74:
18) &, int, double, char, void>' requested here
  type_visit(visitor, Pack{}); //PackTags{});

Comments from Reviewable

ggould-tri · 2018-01-03T15:49:39Z

Reviewed 2 of 2 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

jwnimmer-tri · 2018-01-05T15:08:56Z

Reviewed 1 of 3 files at r2, 1 of 2 files at r3, 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

common/type_util.h, line 3 at r4 (raw file):

#pragma once

/// @file

FYI The name util (as in type_util.h here) always grinds my gears. It's a meaningless word. I wonder if we can do better?

Maybe parameter_pack.h?

common/type_util.h, line 114 at r4 (raw file):

/// Useful for iterating over `type_tag`, `type_pack`, `std::integral_constant`,
/// etc.
struct visit_with_default {

I was okay having type_at and stuff in Drake's global namespace, but the visit, check_always_true etc seem like a bit of a stretch, pollution-wise?

common/type_util.h, line 161 at r4 (raw file):

          typename Visitor = void,
          typename ... Ts>
inline void type_visit(

I think inline is meaningless here?

Comments from Reviewable

EricCousineau-TRI · 2018-01-05T19:46:51Z

Review status: 0 of 3 files reviewed at latest revision, 3 unresolved discussions.

common/type_pack.h, line 3 at r4 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

FYI The name util (as in type_util.h here) always grinds my gears. It's a meaningless word. I wonder if we can do better?

Maybe parameter_pack.h?

Named it to type_pack, since that's one of the main things it provides. Would that be OK, or is that too specific to one class?

common/type_pack.h, line 114 at r4 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I was okay having type_at and stuff in Drake's global namespace, but the visit, check_always_true etc seem like a bit of a stretch, pollution-wise?

Prepended type_ to these. Is that OK, or would you like it nestled in a detail namespace?

common/type_pack.h, line 161 at r4 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I think inline is meaningless here?

Done.

Comments from Reviewable

EricCousineau-TRI · 2018-01-05T19:47:22Z

Switched Check structure to Predicate, to support more direct usage of something like std::is_default_constructible, etc.

Review status: 0 of 3 files reviewed at latest revision, 3 unresolved discussions.

Comments from Reviewable

jwnimmer-tri · 2018-01-05T20:27:11Z

My intuition is that this plausibly could will be used in places beyond the python bindings helpers within the next, say, 12 months. But if you think that chance is low, then we should put this in bindings for now until we have a proven second use. We should expect things in common to be used in several tress within the project.

Reviewed 5 of 5 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

EricCousineau-TRI · 2018-01-05T21:12:38Z

I'd say it might be pretty low for now. Went ahead and put it in bindings/pydrake, can bump it to wherever else if it's needed there.

Review status: 0 of 3 files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

jwnimmer-tri · 2018-01-05T21:15:35Z

Reviewed 2 of 7 files at r5, 5 of 6 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

jwnimmer-tri · 2018-01-05T22:00:04Z

Reviewed 1 of 2 files at r7.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.

Comments from Reviewable

EricCousineau-TRI assigned ggould-tri Jan 2, 2018

EricCousineau-TRI assigned jwnimmer-tri Jan 2, 2018

EricCousineau-TRI force-pushed the feature/type_util branch 4 times, most recently from 22d220e to 3d73cbe Compare January 2, 2018 22:15

EricCousineau-TRI force-pushed the feature/type_util branch from 3d73cbe to edcae62 Compare January 3, 2018 15:04

EricCousineau-TRI force-pushed the feature/type_util branch 2 times, most recently from 870bd4c to ea7bf3a Compare January 3, 2018 15:29

EricCousineau-TRI force-pushed the feature/type_util branch from ea7bf3a to e5aaeb1 Compare January 5, 2018 19:45

EricCousineau-TRI force-pushed the feature/type_util branch from e5aaeb1 to 4577029 Compare January 5, 2018 21:11

Add type_pack meta-programming utilities.

9a0863b

EricCousineau-TRI force-pushed the feature/type_util branch from 4577029 to 9a0863b Compare January 5, 2018 21:18

EricCousineau-TRI merged commit 979fed6 into RobotLocomotion:master Jan 5, 2018

EricCousineau-TRI mentioned this pull request Jan 10, 2018

cpp_param: Add ability to map C++ template paramters to Python to enable simple templating. #7732

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `type_util` meta-programming utilities. #7682

Add `type_util` meta-programming utilities. #7682

EricCousineau-TRI commented Jan 2, 2018 •

edited

Loading

EricCousineau-TRI commented Jan 2, 2018

EricCousineau-TRI commented Jan 2, 2018

EricCousineau-TRI commented Jan 2, 2018

ggould-tri commented Jan 2, 2018

EricCousineau-TRI commented Jan 2, 2018

ggould-tri commented Jan 2, 2018

ggould-tri commented Jan 3, 2018

jwnimmer-tri commented Jan 3, 2018

EricCousineau-TRI commented Jan 3, 2018

ggould-tri commented Jan 3, 2018

EricCousineau-TRI commented Jan 3, 2018

ggould-tri commented Jan 3, 2018

jwnimmer-tri commented Jan 5, 2018

EricCousineau-TRI commented Jan 5, 2018

EricCousineau-TRI commented Jan 5, 2018

jwnimmer-tri commented Jan 5, 2018

EricCousineau-TRI commented Jan 5, 2018

jwnimmer-tri commented Jan 5, 2018

jwnimmer-tri commented Jan 5, 2018

Add type_util meta-programming utilities. #7682

Add type_util meta-programming utilities. #7682

Conversation

EricCousineau-TRI commented Jan 2, 2018 • edited Loading

EricCousineau-TRI commented Jan 2, 2018

EricCousineau-TRI commented Jan 2, 2018

EricCousineau-TRI commented Jan 2, 2018

ggould-tri commented Jan 2, 2018

EricCousineau-TRI commented Jan 2, 2018

ggould-tri commented Jan 2, 2018

ggould-tri commented Jan 3, 2018

jwnimmer-tri commented Jan 3, 2018

EricCousineau-TRI commented Jan 3, 2018

ggould-tri commented Jan 3, 2018

EricCousineau-TRI commented Jan 3, 2018

ggould-tri commented Jan 3, 2018

jwnimmer-tri commented Jan 5, 2018

EricCousineau-TRI commented Jan 5, 2018

EricCousineau-TRI commented Jan 5, 2018

jwnimmer-tri commented Jan 5, 2018

EricCousineau-TRI commented Jan 5, 2018

jwnimmer-tri commented Jan 5, 2018

jwnimmer-tri commented Jan 5, 2018

Add `type_util` meta-programming utilities. #7682

Add `type_util` meta-programming utilities. #7682

EricCousineau-TRI commented Jan 2, 2018 •

edited

Loading