Skip to content

Commit

Permalink
Merge pull request #1522 from ApexAI/iox-#1196-fix-useless-casts
Browse files Browse the repository at this point in the history
iox-#1196 Fix useless cast warnings
  • Loading branch information
elfenpiff authored Jul 19, 2022
2 parents dcedc32 + ed056d4 commit f85ee4b
Show file tree
Hide file tree
Showing 32 changed files with 115 additions and 109 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ This should be used only rarely and only in coordination with an iceoryx maintai

!!! note
iceoryx needs to be built as a static library to work with sanitizer flags, which is automatically achieved when using
the script. If you want to use the ${ICEORYX_WARNINGS} then you have to call `find_package(iceoryx_hoofs)` and `include(IceoryxPlatform)`
the script. If you want to use the ${ICEORYX_CXX_WARNINGS} then you have to call `find_package(iceoryx_hoofs)` and `include(IceoryxPlatform)`
to make use of the ${ICEORYX_SANITIZER_FLAGS}.

## iceoryx library build
Expand Down
19 changes: 10 additions & 9 deletions iceoryx_hoofs/cmake/IceoryxPackageHelper.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -219,20 +219,17 @@ Macro(iox_add_executable)
)
endif()

set(IOX_WARNINGS ${ICEORYX_WARNINGS})
if ( IOX_USE_C_LANGUAGE )
if("-Wno-noexcept-type" IN_LIST IOX_WARNINGS)
list(REMOVE_ITEM IOX_WARNINGS "-Wno-noexcept-type")
endif()
endif()

if ( IOX_USE_C_LANGUAGE )
iox_set_file_language( USE_C_LANGUAGE FILES ${IOX_FILES} )
else()
iox_set_file_language( FILES ${IOX_FILES} )
endif()

target_compile_options(${IOX_TARGET} PRIVATE ${IOX_WARNINGS} ${ICEORYX_SANITIZER_FLAGS})
if ( IOX_USE_C_LANGUAGE )
target_compile_options(${IOX_TARGET} PRIVATE ${ICEORYX_C_WARNINGS} ${ICEORYX_SANITIZER_FLAGS})
else()
target_compile_options(${IOX_TARGET} PRIVATE ${ICEORYX_CXX_WARNINGS} ${ICEORYX_SANITIZER_FLAGS})
endif()

if ( IOX_STACK_SIZE )
if(WIN32)
Expand Down Expand Up @@ -324,7 +321,11 @@ Macro(iox_add_library)

set_target_properties( ${IOX_TARGET} PROPERTIES POSITION_INDEPENDENT_CODE ON )

target_compile_options(${IOX_TARGET} PRIVATE ${ICEORYX_WARNINGS} ${ICEORYX_SANITIZER_FLAGS})
if ( IOX_USE_C_LANGUAGE )
target_compile_options(${IOX_TARGET} PRIVATE ${ICEORYX_C_WARNINGS} ${ICEORYX_SANITIZER_FLAGS})
else()
target_compile_options(${IOX_TARGET} PRIVATE ${ICEORYX_CXX_WARNINGS} ${ICEORYX_SANITIZER_FLAGS})
endif()
target_link_libraries(${IOX_TARGET} PUBLIC ${IOX_PUBLIC_LIBS} PRIVATE ${IOX_PRIVATE_LIBS})
target_include_directories(${IOX_TARGET} PUBLIC ${IOX_PUBLIC_INCLUDES} PRIVATE ${IOX_PRIVATE_INCLUDES})

Expand Down
19 changes: 14 additions & 5 deletions iceoryx_hoofs/cmake/IceoryxPlatform.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,28 @@ else()
endif()

if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
set(ICEORYX_WARNINGS PRIVATE ${ICEORYX_WARNINGS} /W0) # TODO iox-#33 set to /W1
set(ICEORYX_C_WARNINGS PRIVATE /W0) # TODO iox-#33 set to /W1
set(ICEORYX_CXX_WARNINGS PRIVATE ${ICEORYX_C_WARNINGS})
# todo: '/O2' and '/RTC1' (set by default) options are incompatible,
elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
set(ICEORYX_WARNINGS PRIVATE ${ICEORYX_WARNINGS} -W -Wall -Wextra -Wuninitialized -Wpedantic -Wstrict-aliasing -Wcast-align -Wno-noexcept-type -Wconversion)
set(ICEORYX_C_WARNINGS PRIVATE -W -Wall -Wextra -Wuninitialized -Wpedantic -Wstrict-aliasing -Wcast-align -Wconversion)
set(ICEORYX_CXX_WARNINGS PRIVATE ${ICEORYX_C_WARNINGS} -Wno-noexcept-type)

if(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
set(ICEORYX_CXX_WARNINGS PRIVATE ${ICEORYX_CXX_WARNINGS} -Wuseless-cast)
endif()
endif()

if(BUILD_STRICT)
if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
set(ICEORYX_WARNINGS ${ICEORYX_WARNINGS} /W0) # TODO iox-#33 set to /WX
set(ICEORYX_C_WARNINGS ${ICEORYX_C_WARNINGS} /W0)
set(ICEORYX_CXX_WARNINGS ${ICEORYX_CXX_WARNINGS} /W0) # TODO iox-#33 set to /WX
elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
set(ICEORYX_WARNINGS ${ICEORYX_WARNINGS} -Werror)
set(ICEORYX_C_WARNINGS ${ICEORYX_C_WARNINGS} -Werror)
set(ICEORYX_CXX_WARNINGS ${ICEORYX_CXX_WARNINGS} -Werror)
elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
set(ICEORYX_WARNINGS ${ICEORYX_WARNINGS} -Werror)
set(ICEORYX_C_WARNINGS ${ICEORYX_C_WARNINGS} -Werror)
set(ICEORYX_CXX_WARNINGS ${ICEORYX_CXX_WARNINGS} -Werror)
endif()
endif()

Expand Down
27 changes: 16 additions & 11 deletions iceoryx_hoofs/include/iceoryx_hoofs/cxx/attributes.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,28 @@ namespace iox
{
namespace cxx
{
namespace internal
{
/// We use this as an alternative to "static_cast<void>(someVar)" to signal the
/// compiler an unused variable. "static_cast" produces an useless-cast warning
/// on gcc and this approach solves it cleanly.
template <typename T>
inline void IOX_DISCARD_RESULT_IMPL(T&&) noexcept
{
}
} // namespace internal

// NOLINTJUSTIFICATION cannot be implemented with a function, required as inline code
// NOLINTBEGIN(cppcoreguidelines-macro-usage)

/// @brief if a function has a return value which you do not want to use then you can wrap the function with that macro.
/// Purpose is to suppress the unused compiler warning by adding an attribute to the return value
/// @param[in] expr name of the function where the return value is not used.
/// @code
/// uint32_t foo();
/// IOX_DISCARD_RESULT(foo()); // suppress compiler warning for unused return value
/// @endcode
// NOLINTNEXTLINE
#define IOX_DISCARD_RESULT(expr) static_cast<void>(expr)
#define IOX_DISCARD_RESULT(expr) ::iox::cxx::internal::IOX_DISCARD_RESULT_IMPL(expr)

/// @brief IOX_NO_DISCARD adds the [[nodiscard]] keyword if it is available for the current compiler.
/// If additionally the keyword [[gnu::warn_unused]] is present it will be added as well.
Expand All @@ -38,17 +51,13 @@ namespace cxx
/// activate keywords for gcc>=5 or clang>=4
#if defined(_WIN32)
// On WIN32 we are using C++17 which makes the keyword [[nodiscard]] available
// NOLINTNEXTLINE
#define IOX_NO_DISCARD [[nodiscard]]
#elif defined(__APPLE__) && defined(__clang__)
// On APPLE we are using C++17 which makes the keyword [[nodiscard]] available
// NOLINTNEXTLINE
#define IOX_NO_DISCARD [[nodiscard, gnu::warn_unused]]
#elif (defined(__clang__) && __clang_major__ >= 4)
// NOLINTNEXTLINE
#define IOX_NO_DISCARD [[gnu::warn_unused]]
#elif (defined(__GNUC__) && __GNUC__ >= 5)
// NOLINTNEXTLINE
#define IOX_NO_DISCARD [[nodiscard, gnu::warn_unused]]
#else
// on an unknown platform we use for now nothing since we do not know what is supported there
Expand All @@ -62,16 +71,13 @@ namespace cxx
/// activate keywords for gcc>=7 or clang>=4
#if defined(_WIN32)
// On WIN32 we are using C++17 which makes the keyword [[fallthrough]] available
// NOLINTNEXTLINE
#define IOX_FALLTHROUGH [[fallthrough]]
#elif defined(__APPLE__) && defined(__clang__)
// On APPLE we are using C++17 which makes the keyword [[fallthrough]] available
// NOLINTNEXTLINE
#define IOX_FALLTHROUGH [[fallthrough]]
// with C++17 fallthrough was introduced and we can use it
#elif __cplusplus >= 201703L
// clang prints a warning therefore we exclude it here
// NOLINTNEXTLINE
#define IOX_FALLTHROUGH [[fallthrough]]
#elif (defined(__GNUC__) && __GNUC__ >= 7) && !defined(__clang__)
#define IOX_FALLTHROUGH [[gnu::fallthrough]]
Expand All @@ -85,18 +91,17 @@ namespace cxx
/// @note
/// activate attribute for gcc or clang
#if defined(__GNUC__) || defined(__clang__)
// NOLINTNEXTLINE
#define IOX_MAYBE_UNUSED [[gnu::unused]]
#elif defined(_WIN32)
// On WIN32 we are using C++17 which makes the attribute [[maybe_unused]] available
// NOLINTNEXTLINE
#define IOX_MAYBE_UNUSED [[maybe_unused]]
// on an unknown platform we use for now nothing since we do not know what is supported there
#else
#define IOX_MAYBE_UNUSED
#endif


// NOLINTEND(cppcoreguidelines-macro-usage)
} // namespace cxx
} // namespace iox

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class LoFFLi
/// Calculates the required memory size for a free-list
/// @param [in] capacity is the number of elements of the free-list
/// @return the required memory size for a free-list with the requested capacity
static inline constexpr std::size_t requiredIndexMemorySize(const uint32_t capacity) noexcept;
static inline constexpr uint64_t requiredIndexMemorySize(const uint64_t capacity) noexcept;
};

} // namespace concurrent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ namespace iox
{
namespace concurrent
{
inline constexpr std::size_t LoFFLi::requiredIndexMemorySize(const uint32_t capacity) noexcept
inline constexpr uint64_t LoFFLi::requiredIndexMemorySize(const uint64_t capacity) noexcept
{
return (static_cast<size_t>(capacity) + 1U) * sizeof(LoFFLi::Index_t);
return (capacity + 1U) * sizeof(LoFFLi::Index_t);
}

} // namespace concurrent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ inline void PeriodicTask<T>::run() noexcept
posix::SemaphoreWaitState waitState = posix::SemaphoreWaitState::NO_TIMEOUT;
do
{
IOX_DISCARD_RESULT(m_callable());
m_callable();

/// @todo use a refactored posix::Timer::wait method returning TIMER_TICK and TIMER_STOPPED once available
auto waitResult = m_stop->timedWait(m_interval);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ constexpr void* static_storage<Capacity, Align>::allocate(const uint64_t align,
return nullptr; // cannot allocate, already in use
}

auto space = static_cast<size_t>(Capacity);
size_t space = Capacity;
m_ptr = m_bytes;
if (std::align(align, size, m_ptr, space) != nullptr)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ class MessageQueue : public DesignPattern::Creation<MessageQueue, IpcChannelErro
public:
static constexpr mqd_t INVALID_DESCRIPTOR = std::numeric_limits<mqd_t>::max();
static constexpr int32_t ERROR_CODE = -1;
static constexpr size_t SHORTEST_VALID_QUEUE_NAME = 2;
static constexpr size_t NULL_TERMINATOR_SIZE = 1;
static constexpr size_t MAX_MESSAGE_SIZE = 4096;
static constexpr uint64_t SHORTEST_VALID_QUEUE_NAME = 2;
static constexpr uint64_t NULL_TERMINATOR_SIZE = 1;
static constexpr uint64_t MAX_MESSAGE_SIZE = 4096;

/// for calling private constructor in create method
friend class DesignPattern::Creation<MessageQueue, IpcChannelError>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
#define MAP_SHARED 0x01
#define MAP_PRIVATE 0x02
#define MAP_FIXED 0x10
#define MAP_FAILED 1
#define MAP_FAILED (void*)-1
#define PROT_NONE 0
#define PROT_READ 3
#define PROT_WRITE 4
Expand Down
4 changes: 1 addition & 3 deletions iceoryx_hoofs/source/posix_wrapper/access_control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,7 @@ AccessController::createACL(const int32_t numEntries) noexcept
cxx::Ensures(!aclFreeCall.has_error() && "Could not free ACL memory");
};

/// NOLINTJUSTIFICATION required to restore type of the acquired acl_t pointer
/// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
return cxx::success<smartAclPointer_t>(reinterpret_cast<acl_t>(aclInitCall->value), freeACL);
return cxx::success<smartAclPointer_t>(aclInitCall->value, freeACL);
}

bool AccessController::addUserPermission(const Permission permission, const PosixUser::userName_t& name) noexcept
Expand Down
8 changes: 4 additions & 4 deletions iceoryx_hoofs/source/posix_wrapper/message_queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ cxx::expected<IpcChannelError> MessageQueue::destroy() noexcept

cxx::expected<IpcChannelError> MessageQueue::send(const std::string& msg) const noexcept
{
const size_t messageSize = static_cast<size_t>(msg.size()) + NULL_TERMINATOR_SIZE;
if (messageSize > static_cast<size_t>(m_attributes.mq_msgsize))
const uint64_t messageSize = msg.size() + NULL_TERMINATOR_SIZE;
if (messageSize > static_cast<uint64_t>(m_attributes.mq_msgsize))
{
return cxx::error<IpcChannelError>(IpcChannelError::MESSAGE_TOO_LONG);
}
Expand Down Expand Up @@ -303,8 +303,8 @@ cxx::expected<std::string, IpcChannelError> MessageQueue::timedReceive(const uni
cxx::expected<IpcChannelError> MessageQueue::timedSend(const std::string& msg,
const units::Duration& timeout) const noexcept
{
const size_t messageSize = static_cast<size_t>(msg.size()) + NULL_TERMINATOR_SIZE;
if (messageSize > static_cast<size_t>(m_attributes.mq_msgsize))
const uint64_t messageSize = msg.size() + NULL_TERMINATOR_SIZE;
if (messageSize > static_cast<uint64_t>(m_attributes.mq_msgsize))
{
std::cerr << "the message \"" << msg << "\" which should be sent to the message queue \"" << m_name
<< "\" is too long" << std::endl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ cxx::expected<MemoryMap, MemoryMapError> MemoryMapBuilder::create() noexcept
m_fileDescriptor,
m_offset)
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast, performance-no-int-to-ptr)
.failureReturnValue(reinterpret_cast<void*>(MAP_FAILED))
.failureReturnValue(MAP_FAILED)
.evaluate();

if (result)
Expand Down
Loading

0 comments on commit f85ee4b

Please sign in to comment.