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.

Sign up for GitHub

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

Jump to bottom

Make any_internal::FastTypeId() and IdForType() constexpr #28

Merged
merged 3 commits into from
Oct 12, 2017
Merged

Make any_internal::FastTypeId() and IdForType() constexpr #28

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
89f1f53
Make any_internal::FastTypeId() and IdForType() constexpr
Oct 1, 2017
b7bdd3a
Removed unused type_id() function
Oct 10, 2017
bbf8305
Wrap comment at 80 cols
Oct 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 18 additions & 21 deletions absl/types/any.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,23 +94,20 @@ namespace absl {

namespace any_internal {

// FastTypeId<Type>() evaluates at compile/link-time to a unique integer for the
// passed in type. Their values are neither contiguous nor small, making them
// unfit for using as an index into a vector, but a good match for keys into
// maps or straight up comparisons.
// Note that on 64-bit (unix) systems size_t is 64-bit while int is 32-bit and
// the compiler will happily and quietly assign such a 64-bit value to a
// 32-bit integer. While a client should never do that it SHOULD still be safe,
// assuming the BSS segment doesn't span more than 4GiB.
template <typename Type>
struct TypeTag {
constexpr static char dummy_var = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

If Abseil supports C++17 inline variables, this should be that when available.

Copy link
Contributor

Choose a reason for hiding this comment

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

We do not yet branch for C++14 and C++17, but doing so has been talked about.

};

template <typename Type>
constexpr char TypeTag<Type>::dummy_var;

// FastTypeId<Type>() evaluates at compile/link-time to a unique pointer for the
// passed in type. These are meant to be good match for keys into maps or straight
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment exceeds 80 columns.

// up comparisons.
template<typename Type>
inline size_t FastTypeId() {
static_assert(sizeof(char*) <= sizeof(size_t),
"ptr size too large for size_t");

// This static variable isn't actually used, only its address, so there are
// no concurrency issues.
static char dummy_var;
return reinterpret_cast<size_t>(&dummy_var);
constexpr inline const void* FastTypeId() {
return &TypeTag<Type>::dummy_var;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you have a constexpr std::addressof here, you'll want that to avoid ADL on the template parameter Type.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no ADL here since dummy_var is always of type char, so std::addressof does not need to be used. The type Type does not matter. Side note: We do not have a portable constexpr std::addressof as it cannot be implemented without intrinsics. It might be possible for us to implement per supported configuration, but that has not yet been investigated and I suspect will only work for some.

Copy link
Contributor

Choose a reason for hiding this comment

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

@calabrese-google Ah, sorry my mistake. I just saw the template parameter and reacted.

FYI here is the detection libc++ uses for __builtin_addressof.

#if !__has_builtin(__builtin_addressof) && _GNUC_VER < 700
# define _LIBCPP_HAS_NO_BUILTIN_ADDRESSOF
#endif```

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. We'll likely do this at some point.

}

} // namespace any_internal
Expand Down Expand Up @@ -382,7 +379,7 @@ class any {
public:
virtual ~ObjInterface() = default;
virtual std::unique_ptr<ObjInterface> Clone() const = 0;
virtual size_t type_id() const noexcept = 0;
virtual const void* ObjTypeId() const noexcept = 0;
#if ABSL_ANY_DETAIL_HAS_RTTI
virtual const std::type_info& Type() const noexcept = 0;
#endif // ABSL_ANY_DETAIL_HAS_RTTI
Expand All @@ -400,7 +397,7 @@ class any {
return std::unique_ptr<ObjInterface>(new Obj(in_place, value));
}

size_t type_id() const noexcept final { return IdForType<T>(); }
const void* ObjTypeId() const noexcept final { return IdForType<T>(); }

#if ABSL_ANY_DETAIL_HAS_RTTI
const std::type_info& Type() const noexcept final { return typeid(T); }
Expand All @@ -415,16 +412,16 @@ class any {
}

template <typename T>
static size_t IdForType() {
constexpr static const void* IdForType() {
// Note: This type dance is to make the behavior consistent with typeid.
using NormalizedType =
typename std::remove_cv<typename std::remove_reference<T>::type>::type;

return any_internal::FastTypeId<NormalizedType>();
}

size_t GetObjTypeId() const {
return obj_ == nullptr ? any_internal::FastTypeId<void>() : obj_->type_id();
const void* GetObjTypeId() const {
return obj_ ? obj_->ObjTypeId() : any_internal::FastTypeId<void>();
}

// `absl::any` nonmember functions //
Expand Down