Skip to content

Commit

Permalink
qmake/cmake bugfix/enhanced constants selection
Browse files Browse the repository at this point in the history
  • Loading branch information
silverqx committed Jul 8, 2024
1 parent b61da72 commit ffd9dfa
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 24 deletions.
10 changes: 10 additions & 0 deletions NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1522,6 +1522,16 @@ KCachegrind legend:
inline constants:
-----------------

- differences between qmake and CMake build system related to inline/extern constants:
- only one difference is that CMake build system doesn't allow/show the INLINE_CONSTANTS
CMake feature option for shared builds BUILD_SHARED_LIBS=ON even if extern constants work
with static linkage with some compilers, I will have to revisit this in the future
- qmake allows selecting extern_constants even with CONFIG+=static build (static archive library
and/with static linkage)
- the reason for this difference is that the qmake build system is supposed to be used
in development so is good to have this variability, CMake is for production use so it's
logical not to allow this case

- cmake
- normal behavior is that user can switch between inline and extern constants using the INLINE_CONSTANTS cmake option
- default is to provide/show this INLINE_CONSTANTS cmake option with the default value OFF, so extern constants are enabled by default (Clang <v18 has custom patched logic still)
Expand Down
8 changes: 4 additions & 4 deletions cmake/Modules/TinyInitDefaultVariables.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -320,12 +320,12 @@ $<SHELL_PATH:${${TinyOrm_ns}_BINARY_DIR}/tests/${TinyUtils_ns}>${TINY_PATH_SEPAR
endif()

# Specifies which global constant types will be used
if(INLINE_CONSTANTS)
set(tinyExternConstants OFF)
message(VERBOSE "Using inline constants")
else()
if(BUILD_SHARED_LIBS AND NOT INLINE_CONSTANTS)
set(tinyExternConstants ON)
message(VERBOSE "Using extern constants")
else()
set(tinyExternConstants OFF)
message(VERBOSE "Using inline constants")
endif()
set(TINY_EXTERN_CONSTANTS ${tinyExternConstants} CACHE INTERNAL
"Determine whether ${TinyOrm_target} library will be built with extern or inline \
Expand Down
55 changes: 40 additions & 15 deletions cmake/Modules/TinyOptions.cmake
Original file line number Diff line number Diff line change
@@ -1,39 +1,64 @@
include(TinyFeatureOptions)

# Initialize INLINE_CONSTANTS CMake feature dependent option.
# MinGW Clang shared build crashes with inline constants (fixed in Clang v18).
# Clang-cl shared build crashes with extern constants so force to inline constants 😕🤔
# (also fixed in Clang v18), only one option with the Clang-cl is inline constants
# for both shared/static builds.
# MinGW Clang <18 shared build crashes with inline constants (fixed in Clang v18).
# Clang-cl <18 shared build crashes with extern constants so force to inline constants
# (also fixed in Clang v18) 😕🤔, only one option with the Clang-cl is inline constants
# for both shared/static builds (not true from Clang v18).
# Look at NOTES.txt[inline constants] how this funckin machinery works. 😎
# Related issue: https://github.com/llvm/llvm-project/issues/55938
# Possible reason why extern constants don't work with static linking (already fixed,
# it started working mysteriously 😅):
# Possible reason why extern constants don't work with static linking (fixed in some
# cases, but doesn't always work):
# https://stackoverflow.com/a/3187324/2266467
function(tiny_initialize_inline_constants_option)
macro(tiny_initialize_inline_constants_option)

# Summary how this works:
# - INLINE_CONSTANT feature option is available for shared builds only so you can
# select if you want to use inline or extern constants
# - INLINE_CONSTANT isn't available:
# - for static builds, in this case is always forced to ON (inline)
# - for MinGW Clang <18 shared build, in this case is always forced to OFF (extern)
# because shared build crashes with inline constants
# - Clang-cl <18 shared build, in this case is always forced to ON (inline)
# because shared build crashes with extern constants
#
# Next, based on the BUILD_SHARED_LIBS and INLINE_CONSTANTS values will be decided
# if inline or extern constants will be used in tiny_init_tiny_variables() that sets
# the TINY_EXTERN_CONSTANTS INTERNAL CACHE CMake variable and based on this variable
# will be set TINYORM_EXTERN_CONSTANTS or TINYORM_INLINE_CONSTANTS
# -D compiler definition later in the main CMakeLists.txt. 😅
# Also, based on the TINY_EXTERN_CONSTANTS will be decided which source files
# for inline/extern constants will be used constants_extern/_p.hpp/.cpp or
# constants_inline/_p.hpp for in all projects.

# MinGW Clang <18 shared build crashes with inline constants so force extern constants
# if <depends>=FALSE
if(MINGW AND BUILD_SHARED_LIBS AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND
CMAKE_CXX_COMPILER_VERSION VERSION_LESS "18"
)
set(tinyInlineConstantsForceValue OFF)
# All other cases if <depends>=FALSE force the INLINE_CONSTANTS to ON
else()
set(tinyInlineConstantsForceValue ON)
endif()

# Default value for shared builds is OFF and for static builds ON (with exceptions
# described all around)
feature_option_dependent(INLINE_CONSTANTS
"Use inline constants instead of extern constants in the shared build. \
OFF is highly recommended for the shared build; is always ON for the static build"
# Default value for shared builds is OFF and for static builds ON
OFF
# Always TRUE for Clang >18 and for everything else
"NOT (CMAKE_CXX_COMPILER_ID STREQUAL \"Clang\" AND \
CMAKE_CXX_COMPILER_VERSION VERSION_LESS \"18\" AND \
(MSVC OR (MINGW AND BUILD_SHARED_LIBS)))"
# When the above condition is FALSE then always force INLINE_CONSTANTS to ON excepting
# for MinGW Clang <18 shared build, in this case we need extern constants 😅
# Always TRUE if BUILD_SHARED_LIBS=ON excluding Clang-cl <18 with MSVC or
# MinGW Clang <18
"BUILD_SHARED_LIBS AND NOT ((MSVC OR MINGW) AND \
CMAKE_CXX_COMPILER_ID STREQUAL \"Clang\" AND \
CMAKE_CXX_COMPILER_VERSION VERSION_LESS \"18\")"
# When the above <depends> condition is FALSE then always force
# the INLINE_CONSTANTS to ON except for MinGW Clang <18 shared build,
# in this case we need extern constants 😅 (so force INLINE_CONSTANT to OFF)
${tinyInlineConstantsForceValue}
)

unset(tinyInlineConstantsForceValue)

endfunction()
endmacro()
10 changes: 8 additions & 2 deletions include/orm/config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,16 @@ TINY_SYSTEM_HEADER
#if defined(TINYORM_EXTERN_CONSTANTS) && defined(TINYORM_INLINE_CONSTANTS)
# error Both TINYORM_EXTERN_CONSTANTS and TINYORM_INLINE_CONSTANTS defined.
#endif
/* Enforce extern constants if nothing is defined.
/* Enforce inline/extern constants if neither is defined:
- extern constants for shared builds
- inline constants for static builds
Look at NOTES.txt[inline constants] how this funckin machinery works. 😎 */
#if !defined(TINYORM_EXTERN_CONSTANTS) && !defined(TINYORM_INLINE_CONSTANTS)
# define TINYORM_EXTERN_CONSTANTS
# if defined(TINYORM_BUILDING_SHARED) || defined(TINYORM_LINKING_SHARED)
# define TINYORM_EXTERN_CONSTANTS
# else
# define TINYORM_INLINE_CONSTANTS
# endif
#endif

// Check
Expand Down
13 changes: 10 additions & 3 deletions qmake/common/common.pri
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,18 @@ CONFIG(release, debug|release): \
# TinyORM configuration
# ---

# Use extern constants by default
# Look at NOTES.txt[inline constants] how this funckin machinery works 😎
!extern_constants: \
!inline_constants: \
CONFIG += extern_constants
!inline_constants {
# Use extern constants by default for shared builds
CONFIG(shared, dll|shared|static|staticlib) | \
CONFIG(dll, dll|shared|static|staticlib): \
CONFIG += extern_constants

# Archive library build (static linkage)
else: \
CONFIG += inline_constants
}

# Platform specific configuration
# ---
Expand Down

0 comments on commit ffd9dfa

Please sign in to comment.