From ffd9dfafdd50ea94c8a5863f5f974cf107d8f20a Mon Sep 17 00:00:00 2001 From: silverqx Date: Mon, 8 Jul 2024 14:30:03 +0200 Subject: [PATCH] qmake/cmake bugfix/enhanced constants selection --- NOTES.txt | 10 ++++ cmake/Modules/TinyInitDefaultVariables.cmake | 8 +-- cmake/Modules/TinyOptions.cmake | 55 ++++++++++++++------ include/orm/config.hpp | 10 +++- qmake/common/common.pri | 13 +++-- 5 files changed, 72 insertions(+), 24 deletions(-) diff --git a/NOTES.txt b/NOTES.txt index b80a98de1..f3a8023d0 100644 --- a/NOTES.txt +++ b/NOTES.txt @@ -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 ${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 \ diff --git a/cmake/Modules/TinyOptions.cmake b/cmake/Modules/TinyOptions.cmake index 98aaaa16d..df2815e79 100644 --- a/cmake/Modules/TinyOptions.cmake +++ b/cmake/Modules/TinyOptions.cmake @@ -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 =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 =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 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() diff --git a/include/orm/config.hpp b/include/orm/config.hpp index 1f7f41679..cf7fa77c8 100644 --- a/include/orm/config.hpp +++ b/include/orm/config.hpp @@ -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 diff --git a/qmake/common/common.pri b/qmake/common/common.pri index 41cafed33..3cd5d4416 100644 --- a/qmake/common/common.pri +++ b/qmake/common/common.pri @@ -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 # ---