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

Increase verbosity of size static asserts #256

Merged
merged 1 commit into from
Jul 24, 2023

Conversation

JakeHillion
Copy link
Contributor

Summary

Size static asserts currently tell you the type and the expected size in the log. This is a bit limited as the only bit of invisible information there is the actual size. Layer the static assert behind a struct so the error message is more descriptive.

Annoyingly this does make the message more verbose (3 "lines" of errors rather than 1). I think we can get it back down to 1 if a macro is used to construct a custom static assert message, but this way seemed more C++ friendly.

Test plan

  • CI

New example:

/tmp/oid-cache/981275032686841092.cc:154:5: error: static_assert failed due to requirement '24UL == 32UL'
    static_assert(ExpectedSize == ActualSize);
    ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/oid-cache/981275032686841092.cc:159:5: note: in instantiation of template class 'validate_size<folly::sorted_vector_map<unsigned int, std::vector<long>, OIInternal::(anonymous namespace)::less_24>, 24, 32>' requested here
  : validate_size< Type, ExpectedSize, sizeof( Type ) > {};
    ^
/tmp/oid-cache/981275032686841092.cc:4760:15: note: in instantiation of template class 'validate_size<folly::sorted_vector_map<unsigned int, std::vector<long>, OIInternal::(anonymous namespace)::less_24>, 24, 0>' requested here
static_assert(validate_size<folly::sorted_vector_map<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>, less_24, std::allocator<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>>, void, std::vector<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>, std::allocator<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>>>>, 24>::value);

Shows the two numbers in the first static assert. The middle "note" gives all of the information in one place (name, actual size, expected size). The bottom one is useless.

Old example:

/tmp/oid-cache/981275032686841092.cc:4751:1: error: static_assert failed due to requirement 'sizeof(folly::sorted_vector_map<unsigned int, std::vector<long, std::allocator<long>>, OIInternal::(anonymous namespace)::less_24, std::allocator<std::pair<unsigned int, std::vector<long, std::allocator<long>>>>, void, std::vector<std::pair<unsigned int, std::vector<long, std::allocator<long>>>, std::allocator<std::pair<unsigned int, std::vector<long, std::allocator<long>>>>>>) == 2
4' "Unexpected size of container folly::sorted_vector_map<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>, less_24, std::allocator<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>>, void, std::vector<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>, std::allocator<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>>>>"
static_assert(sizeof(folly::sorted_vector_map<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>, less_24, std::allocator<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>>, void, std::vector<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>, std::allocator<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>>>>) == 24, "Unexpected size of container folly::sorted_vector_map<ClusterType_23, std::vec
tor<int64_t, std::allocator<int64_t>>, less_24, std::allocator<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>>, void, std::vector<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>, std::allocator<std::pair<ClusterType_23, std::vector<int64_t, std::allocator<int64_t>>>>>>");
^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Copy link
Contributor

@ajor ajor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! I wanted to have output like this ages ago (it's so much more useful) but couldn't work out how to do it.

Can we do the same for the offsetof static asserts?

oi/CodeGen.cpp Show resolved Hide resolved
@JakeHillion
Copy link
Contributor Author

Added support for offsets too:

/tmp/oid-cache/981275032686841092.cc:163:3: error: static_assert failed due to requirement '24UL == 16UL'
  static_assert(ExpectedOffset == ActualOffset);
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/oid-cache/981275032686841092.cc:5299:15: note: in instantiation of template class 'validate_offset<24, 16>' requested here
static_assert(validate_offset<offsetof(Foo_33, bar_2), 16>::value, "Unexpected offset of Foo_33::bar_2");

@JakeHillion JakeHillion merged commit 623f896 into facebookexperimental:main Jul 24, 2023
@JakeHillion JakeHillion deleted the static-asserts branch July 24, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants