Skip to content

Instantly share code, notes, and snippets.

@ecatmur
Last active March 14, 2022 15:00
Show Gist options
  • Save ecatmur/67fb43a2e8a3521ac34a9c2d4faabc69 to your computer and use it in GitHub Desktop.
Save ecatmur/67fb43a2e8a3521ac34a9c2d4faabc69 to your computer and use it in GitHub Desktop.
functional cast as direct initialization

https://github.com/gcc-mirror/gcc/compare/master...ecatmur:functional-cast-direct-init

Rationale:

Consider:

#include <cstdint>
std::uintptr_t f();
std::uintptr_t f() { return std::uintptr_t("hello"); }

Even though this is clearly unsafe and quite possibly unintended, there is no combination of flags on any of the major 4 compilers that will induce them to emit a warning. Not even '-Wold-style-cast' will help, since this is not an old-style (i.e. C-style) cast; it is a functional cast (that is interpreted as a C-style cast).

Similarly, https://cplusplus.github.io/LWG/issue3528 is a welcome fix for an insidious bug. However, the resolution (requiring is_constructible, i.e., that direct-initialization be well-formed) goes beyond the justification for the issue (since there are conversions allowed by static_cast that are not available to direct-initialization), leaves open the possibility that there could be code where direct-initialization is well-formed but has different semantics to those selected by cast notation, and leaves users vulnerable to committing the same error in their own libraries.

For example, static_cast<int&&>(declval<int&>()) is valid, but int&& t(declval<int&>()); is not; likewise base-to-derived pointer and reference conversions, enumeration conversions, and so forth.

Instead, we propose deprecating the cast notation interpretation of the functional notation entirely; that is, we wish to strike the first sentence of [expr.type.conv]/2. Per this change, functional notation will be fully consistent with direct-initialization, except that it will also be able to construct void prvalues (but only with an initializer of () or {}).

https://github.com/gcc-mirror/gcc/compare/master...ecatmur:functional-cast-direct-init is a branch of gcc and libstdc++ implementing this change (as a hard error, in C++23 mode) and fixing all detected source files, headers and testcases.

boost.patch below contains fixes to various Boost libraries as used by our core framework. At present, there are 8 fixes of which 6 are static_cast to enumeration type and 2 are reinterpret_cast between pointer and integral type. Notably, one of those reinterpret_cast identifies a bug using std::size_t to store the result of a cast from pointer; this should of course be std::uintptr_t.

diff --git a/src/monotonic_buffer_resource.cpp b/src/monotonic_buffer_resource.cpp
index f625f65..670c5d1 100644
--- a/src/monotonic_buffer_resource.cpp
+++ b/src/monotonic_buffer_resource.cpp
@@ -108,7 +108,7 @@ std::size_t monotonic_buffer_resource::remaining_storage(std::size_t alignment,
{
const uintptr_type up_alignment_minus1 = alignment - 1u;
const uintptr_type up_alignment_mask = ~up_alignment_minus1;
- const uintptr_type up_addr = uintptr_type(m_current_buffer);
+ const uintptr_type up_addr = reinterpret_cast<uintptr_type>(m_current_buffer);
const uintptr_type up_aligned_addr = (up_addr + up_alignment_minus1) & up_alignment_mask;
wasted_due_to_alignment = std::size_t(up_aligned_addr - up_addr);
return m_current_buffer_size <= wasted_due_to_alignment ? 0u : m_current_buffer_size - wasted_due_to_alignment;
diff --git a/src/exceptions.cpp b/src/exceptions.cpp
index d32eb54..82c2409 100644
--- a/src/exceptions.cpp
+++ b/src/exceptions.cpp
@@ -17,7 +17,7 @@ public:
virtual std::string message( int ev) const
{
- switch (BOOST_SCOPED_ENUM_NATIVE(coroutine_errc)(ev))
+ switch (static_cast<BOOST_SCOPED_ENUM_NATIVE(coroutine_errc)>(ev))
{
case coroutine_errc::no_data:
return std::string("Operation not permitted because coroutine "
diff --git a/include/boost/iostreams/device/file_descriptor.hpp b/include/boost/iostreams/device/file_descriptor.hpp
index 5d6af12..c7600b0 100644
--- a/include/boost/iostreams/device/file_descriptor.hpp
+++ b/include/boost/iostreams/device/file_descriptor.hpp
@@ -141,7 +141,7 @@ private:
// open overload taking a detail::path
void open( const detail::path& path,
BOOST_IOS::openmode,
- BOOST_IOS::openmode = BOOST_IOS::openmode(0) );
+ BOOST_IOS::openmode = static_cast<BOOST_IOS::openmode>(0) );
typedef detail::file_descriptor_impl impl_type;
shared_ptr<impl_type> pimpl_;
diff --git a/include/boost/mpl/assert.hpp b/include/boost/mpl/assert.hpp
index a0a5e361..82919cd7 100644
--- a/include/boost/mpl/assert.hpp
+++ b/include/boost/mpl/assert.hpp
@@ -343,7 +343,7 @@ BOOST_MPL_AUX_ASSERT_CONSTANT( \
, BOOST_PP_CAT(mpl_assertion_in_line_,counter) = sizeof( \
boost::mpl::assertion_failed<BOOST_PP_CAT(mpl_assert_rel_value,counter)>( \
(boost::mpl::failed ************ ( boost::mpl::assert_relation< \
- boost::mpl::assert_::relations( sizeof( \
+ static_cast<boost::mpl::assert_::relations>( sizeof( \
boost::mpl::assert_::arg rel boost::mpl::assert_::arg \
) ) \
, x \
diff --git a/include/boost/multi_index/detail/ord_index_node.hpp b/include/boost/multi_index/detail/ord_index_node.hpp
index 72bffe3..dd2aeae 100644
--- a/include/boost/multi_index/detail/ord_index_node.hpp
+++ b/include/boost/multi_index/detail/ord_index_node.hpp
@@ -147,7 +147,7 @@ struct ordered_index_node_compressed_base
operator ordered_index_color()const
{
- return ordered_index_color(*r&uintptr_type(1));
+ return static_cast<ordered_index_color>(*r&uintptr_type(1));
}
color_ref& operator=(ordered_index_color c)
diff --git a/src/cmdline.cpp b/src/cmdline.cpp
index a768fa2..4a71518 100644
--- a/src/cmdline.cpp
+++ b/src/cmdline.cpp
@@ -119,7 +119,7 @@ namespace boost { namespace program_options { namespace detail {
style = default_style;
check_style(style);
- this->m_style = style_t(style);
+ this->m_style = static_cast<style_t>(style);
}
void
diff --git a/include/boost/property_tree/detail/rapidxml.hpp b/include/boost/property_tree/detail/rapidxml.hpp
index 5c297de..71aa27e 100644
--- a/include/boost/property_tree/detail/rapidxml.hpp
+++ b/include/boost/property_tree/detail/rapidxml.hpp
@@ -561,7 +561,7 @@ namespace boost { namespace property_tree { namespace detail {namespace rapidxml
char *align(char *ptr)
{
- std::size_t alignment = ((BOOST_PROPERTY_TREE_RAPIDXML_ALIGNMENT - (std::size_t(ptr) & (BOOST_PROPERTY_TREE_RAPIDXML_ALIGNMENT - 1))) & (BOOST_PROPERTY_TREE_RAPIDXML_ALIGNMENT - 1));
+ std::size_t alignment = ((BOOST_PROPERTY_TREE_RAPIDXML_ALIGNMENT - (reinterpret_cast<std::uintptr_t>(ptr) & (BOOST_PROPERTY_TREE_RAPIDXML_ALIGNMENT - 1))) & (BOOST_PROPERTY_TREE_RAPIDXML_ALIGNMENT - 1));
return ptr + alignment;
}
diff --git a/src/future.cpp b/src/future.cpp
index a477e709..95cbba9a 100644
--- a/src/future.cpp
+++ b/src/future.cpp
@@ -33,7 +33,7 @@ namespace boost
std::string
future_error_category::message(int ev) const
{
- switch (BOOST_SCOPED_ENUM_NATIVE(future_errc)(ev))
+ switch (static_cast<BOOST_SCOPED_ENUM_NATIVE(future_errc)>(ev))
{
case future_errc::broken_promise:
return std::string("The associated promise has been destructed prior "
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment