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

Do not define AVIF_ENABLE_NODISCARD for avif.h #2351

Merged
merged 2 commits into from
Aug 2, 2024
Merged

Do not define AVIF_ENABLE_NODISCARD for avif.h #2351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a4654aa
Do not define AVIF_ENABLE_NODISCARD for avif.h
wantehchang Jul 31, 2024
35c4a4e
Require C23 and test __STDC_VERSION__ >= 202000L
wantehchang Aug 1, 2024
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
32 changes: 14 additions & 18 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,19 @@ include(FetchContent)
include(FindPkgConfig)
include(AvifExternalProjectUtils)

option(AVIF_ENABLE_NODISCARD "Add [[nodiscard]] to some functions. CMake must be at least 3.21 to force C23." OFF)

# Set C99 as the default
set(CMAKE_C_STANDARD 99)
if(AVIF_ENABLE_NODISCARD)
# [[nodiscard]] requires C23.
if(CMAKE_VERSION VERSION_LESS 3.21.0)
message(FATAL_ERROR "CMake must be at least 3.21 to force C23, bailing out")
endif()
set(CMAKE_C_STANDARD 23)
set(CMAKE_C_STANDARD_REQUIRED ON)
else()
set(CMAKE_C_STANDARD 99)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would think this would be required too, but it's highly unlikely anyone would try this with a c90 only compiler (or force those flags)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we assume the compiler supports C99.

endif()

# SOVERSION scheme: MAJOR.MINOR.PATCH
# If there was an incompatible interface change:
Expand All @@ -52,7 +63,6 @@ set(LIBRARY_SOVERSION ${LIBRARY_VERSION_MAJOR})
option(BUILD_SHARED_LIBS "Build shared avif library" ON)

option(AVIF_ENABLE_WERROR "Treat all compiler warnings as errors" OFF)
option(AVIF_ENABLE_NODISCARD "Add [[nodiscard]] to some functions. CMake must be at least 3.21 to force C23" OFF)

option(AVIF_ENABLE_EXPERIMENTAL_YCGCO_R "Enable experimental YCgCo-R matrix code" OFF)
option(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP
Expand Down Expand Up @@ -334,22 +344,6 @@ endif()

target_link_libraries(avif_obj PRIVATE avif_enable_warnings)

if(AVIF_ENABLE_NODISCARD)
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.21.0)
set(CMAKE_C_STANDARD 23)
set_property(TARGET avif_obj PROPERTY C_STANDARD 23)
else()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we redefine AVIF_ENABLE_NODISCARD to be a request rather than a requirement, we don't need this else block.

unset(CMAKE_C_STANDARD)
set_property(TARGET avif_obj PROPERTY C_STANDARD)
if(CMAKE_C_COMPILER_ID MATCHES "Clang" OR CMAKE_C_COMPILER_ID MATCHES "GNU")
target_compile_options(avif_obj PUBLIC $<BUILD_INTERFACE:$<$<COMPILE_LANGUAGE:C>:-std=gnu2x>>)
elseif(CMAKE_C_COMPILER_ID MATCHES "MSVC")
target_compile_options(avif_obj PUBLIC $<BUILD_INTERFACE:$<$<COMPILE_LANGUAGE:C>:/std:clatest>>)
endif()
endif()
target_compile_definitions(avif_obj PUBLIC $<BUILD_INTERFACE:AVIF_ENABLE_NODISCARD=1>)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The definition of the AVIF_ENABLE_NODISCARD macro for avif.h is also deleted.

endif()

if(AVIF_ENABLE_COVERAGE)
if(CMAKE_C_COMPILER_ID MATCHES "Clang" OR CMAKE_C_COMPILER_ID MATCHES "GNU")
message(STATUS "libavif: Enabling coverage for Clang")
Expand Down Expand Up @@ -592,7 +586,9 @@ if(AVIF_LIB_USE_CXX OR (AVIF_BUILD_APPS AND AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) O
)
enable_language(CXX)
if(AVIF_ENABLE_NODISCARD)
# [[nodiscard]] requires C++17.
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
else()
set(CMAKE_CXX_STANDARD 14)
endif()
Expand Down
10 changes: 8 additions & 2 deletions include/avif/avif.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,14 @@ extern "C" {
#define AVIF_API
#endif // defined(AVIF_DLL)

#if defined(AVIF_ENABLE_NODISCARD) || (defined(__cplusplus) && __cplusplus >= 201703L) || \
(defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L)
// [[nodiscard]] requires C++17 and C23.
//
// If the -std=c2x or -std=gnu2x option is specified, __STDC_VERSION__ is
// * 202000L in GCC 13.2.0, Clang 16.0.6, and Apple Clang 15.0.0; or
// * 202311L in Clang 19.0.0git.
// If the /std:clatest option is specified, __STDC_VERSION__ is
// * 202312L in Microsoft Visual Studio 17.10.5.
#if (defined(__cplusplus) && __cplusplus >= 201703L) || (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202000L)
#define AVIF_NODISCARD [[nodiscard]]
#else
// Starting with 3.9, clang allows defining the warn_unused_result attribute for enums.
Expand Down
Loading