From ca4f7cfecc1f20d40285a8281bf0c42c2b9d11b3 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 30 Oct 2024 10:31:49 -0700 Subject: [PATCH 1/5] Auto-detect std::string_view support by default Instead of opting in std::string_view support via PUGIXML_STRING_VIEW define, always enable it when C++17 is supported; this still requires enabling C++17 support in the compiler, which this change doesn't attempt to do yet. --- .github/workflows/build.yml | 4 ++-- CMakeLists.txt | 10 ++-------- src/pugiconfig.hpp | 8 +++----- src/pugixml.hpp | 6 +++--- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0b460b7..8d0fddb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -12,7 +12,7 @@ jobs: matrix: os: [ubuntu, macos] compiler: [g++, clang++] - defines: [standard, PUGIXML_WCHAR_MODE, PUGIXML_COMPACT, PUGIXML_NO_EXCEPTIONS, PUGIXML_STRING_VIEW] + defines: [standard, PUGIXML_WCHAR_MODE, PUGIXML_COMPACT, PUGIXML_NO_EXCEPTIONS] exclude: - os: macos compiler: g++ @@ -39,7 +39,7 @@ jobs: strategy: matrix: arch: [Win32, x64] - defines: [standard, PUGIXML_WCHAR_MODE, PUGIXML_COMPACT, PUGIXML_NO_EXCEPTIONS, PUGIXML_STRING_VIEW] + defines: [standard, PUGIXML_WCHAR_MODE, PUGIXML_COMPACT, PUGIXML_NO_EXCEPTIONS] steps: - uses: actions/checkout@v1 - name: cmake configure diff --git a/CMakeLists.txt b/CMakeLists.txt index ede427c..cf43abf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,7 +32,7 @@ set(PUGIXML_BUILD_DEFINES CACHE STRING "Build defines for custom options") separate_arguments(PUGIXML_BUILD_DEFINES) # Technically not needed for this file. This is builtin CMAKE global variable. -option(BUILD_SHARED_LIBS "Build shared instead of static library" OFF) +option(BUILD_SHARED_LIBS "Build shared instead of static library" OFF) # Expose option to build PUGIXML as static as well when the global BUILD_SHARED_LIBS variable is set cmake_dependent_option(PUGIXML_BUILD_SHARED_AND_STATIC_LIBS @@ -48,8 +48,7 @@ option(PUGIXML_INSTALL "Enable installation rules" ON) option(PUGIXML_NO_XPATH "Disable XPath" OFF) option(PUGIXML_NO_STL "Disable STL" OFF) option(PUGIXML_NO_EXCEPTIONS "Disable Exceptions" OFF) -option(PUGIXML_STRING_VIEW "Enable std::string_view overloads" OFF) # requires C++17 and for PUGIXML_NO_STL to be OFF -mark_as_advanced(PUGIXML_NO_XPATH PUGIXML_NO_STL PUGIXML_NO_EXCEPTIONS PUGIXML_STRING_VIEW) +mark_as_advanced(PUGIXML_NO_XPATH PUGIXML_NO_STL PUGIXML_NO_EXCEPTIONS) set(PUGIXML_PUBLIC_DEFINITIONS $<$:PUGIXML_WCHAR_MODE> @@ -57,7 +56,6 @@ set(PUGIXML_PUBLIC_DEFINITIONS $<$:PUGIXML_NO_XPATH> $<$:PUGIXML_NO_STL> $<$:PUGIXML_NO_EXCEPTIONS> - $<$:PUGIXML_STRING_VIEW> ) # This is used to backport a CMake 3.15 feature, but is also forwards compatible @@ -66,10 +64,6 @@ if (NOT DEFINED CMAKE_MSVC_RUNTIME_LIBRARY) MultiThreaded$<$:Debug>$<$>:DLL>) endif() -if (PUGIXML_STRING_VIEW AND (NOT DEFINED CMAKE_CXX_STANDARD OR CMAKE_CXX_STANDARD LESS 17)) - message(WARNING "PUGIXML_STRING_VIEW requires CMAKE_CXX_STANDARD to be set to 17 or later") -endif() - if (NOT DEFINED CMAKE_CXX_STANDARD_REQUIRED) set(CMAKE_CXX_STANDARD_REQUIRED ON) endif() diff --git a/src/pugiconfig.hpp b/src/pugiconfig.hpp index cc138ba..af32925 100644 --- a/src/pugiconfig.hpp +++ b/src/pugiconfig.hpp @@ -46,13 +46,11 @@ // Uncomment this to switch to header-only version // #define PUGIXML_HEADER_ONLY -// Uncomment this to enable long long support +// Uncomment this to enable long long support (usually enabled automatically) // #define PUGIXML_HAS_LONG_LONG -// Uncomment this to enable support for std::string_view (requires c++17 and for PUGIXML_NO_STL to not be set) -// Note: In a future version of pugixml this macro will become obsolete. -// Support will then be enabled automatically if the used C++ standard supports it. -// #define PUGIXML_STRING_VIEW +// Uncomment this to enable support for std::string_view (usually enabled automatically) +// #define PUGIXML_HAS_STRING_VIEW #endif diff --git a/src/pugixml.hpp b/src/pugixml.hpp index f2d985e..b9155d8 100644 --- a/src/pugixml.hpp +++ b/src/pugixml.hpp @@ -38,8 +38,8 @@ # include #endif -// Check if std::string_view is both requested and available -#if defined(PUGIXML_STRING_VIEW) && !defined(PUGIXML_NO_STL) +// Check if std::string_view is available +#if !defined(PUGIXML_HAS_STRING_VIEW) && !defined(PUGIXML_NO_STL) # if __cplusplus >= 201703L # define PUGIXML_HAS_STRING_VIEW # elif defined(_MSVC_LANG) && _MSVC_LANG >= 201703L @@ -231,7 +231,7 @@ namespace pugi // the document; this flag is only recommended for parsing documents with many PCDATA nodes in memory-constrained environments. // This flag is off by default. const unsigned int parse_embed_pcdata = 0x2000; - + // This flag determines whether determines whether the the two pcdata should be merged or not, if no intermediatory data are parsed in the document. // This flag is off by default. const unsigned int parse_merge_pcdata = 0x4000; From d5f14adb3cb852355e7adbaa0deffe280204332e Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 30 Oct 2024 10:36:36 -0700 Subject: [PATCH 2/5] docs: Update documentation to address PUGIXML_HAS_STRING_VIEW changes The PUGIXML_STRING_VIEW define is no longer necessary but _HAS_STRING_VIEW can be enabled manually if the compiler supports it without advertising C++17 support. --- docs/manual.adoc | 2 +- docs/manual.html | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/docs/manual.adoc b/docs/manual.adoc index 79d6106..a6cf07d 100644 --- a/docs/manual.adoc +++ b/docs/manual.adoc @@ -244,7 +244,7 @@ NOTE: In that example `PUGIXML_API` is inconsistent between several source files [[PUGIXML_HAS_LONG_LONG]]`PUGIXML_HAS_LONG_LONG` define enables support for `long long` type in pugixml. This define is automatically enabled if your platform is known to have `long long` support (i.e. has C{plus}{plus}11 support or uses a reasonably modern version of a known compiler); if pugixml does not recognize that your platform supports `long long` but in fact it does, you can enable the define manually. -[[PUGIXML_HAS_STRING_VIEW]]`PUGIXML_HAS_STRING_VIEW` define enables function overloads that accept `std::basic_string_view` arguments. This define is automatically enabled if built targeting C++17 or later AND if `PUGIXML_STRING_VIEW` is also defined. The requirement to additionally define `PUGIXML_STRING_VIEW` will be retired in a future version. +[[PUGIXML_HAS_STRING_VIEW]]`PUGIXML_HAS_STRING_VIEW` define enables function overloads that accept `std::string_view` arguments. This define is automatically enabled if built targeting C{plus}{plus}17 or later; if pugixml does not recognize that your platform supports `std::string_view` but in fact it does, you can enable the define manually. [[install.portability]] === Portability diff --git a/docs/manual.html b/docs/manual.html index c460cf4..0fb0c2d 100644 --- a/docs/manual.html +++ b/docs/manual.html @@ -4,7 +4,7 @@ - + pugixml 1.14 manual @@ -141,7 +141,7 @@ p a>code:hover{color:rgba(0,0,0,.9)} #content::before{content:none} #header>h1:first-child{color:rgba(0,0,0,.85);margin-top:2.25rem;margin-bottom:0} #header>h1:first-child+#toc{margin-top:8px;border-top:1px solid #dddddf} -#header>h1:only-child{border-bottom:1px solid #dddddf;padding-bottom:8px} +#header>h1:only-child,body.toc2 #header>h1:nth-last-child(2){border-bottom:1px solid #dddddf;padding-bottom:8px} #header .details{border-bottom:1px solid #dddddf;line-height:1.45;padding-top:.25em;padding-bottom:.25em;padding-left:.25em;color:rgba(0,0,0,.6);display:flex;flex-flow:row wrap} #header .details span:first-child{margin-left:-.125em} #header .details span.email a{color:rgba(0,0,0,.85)} @@ -163,7 +163,6 @@ p a>code:hover{color:rgba(0,0,0,.9)} #toctitle{color:#7a2518;font-size:1.2em} @media screen and (min-width:768px){#toctitle{font-size:1.375em} body.toc2{padding-left:15em;padding-right:0} -body.toc2 #header>h1:nth-last-child(2){border-bottom:1px solid #dddddf;padding-bottom:8px} #toc.toc2{margin-top:0!important;background:#f8f8f7;position:fixed;width:15em;left:0;top:0;border-right:1px solid #e7e7e9;border-top-width:0!important;border-bottom-width:0!important;z-index:1000;padding:1.25em 1em;height:100%;overflow:auto} #toc.toc2 #toctitle{margin-top:0;margin-bottom:.8rem;font-size:1.2em} #toc.toc2>ul{font-size:.9em;margin-bottom:0} @@ -329,7 +328,7 @@ a.image{text-decoration:none;display:inline-block} a.image object{pointer-events:none} sup.footnote,sup.footnoteref{font-size:.875em;position:static;vertical-align:super} sup.footnote a,sup.footnoteref a{text-decoration:none} -sup.footnote a:active,sup.footnoteref a:active,#footnotes .footnote a:first-of-type:active{text-decoration:underline} +sup.footnote a:active,sup.footnoteref a:active{text-decoration:underline} #footnotes{padding-top:.75em;padding-bottom:.75em;margin-bottom:.625em} #footnotes hr{width:20%;min-width:6.25em;margin:-.25em 0 .75em;border-width:1px 0 0} #footnotes .footnote{padding:0 .375em 0 .225em;line-height:1.3334;font-size:.875em;margin-left:1.2em;margin-bottom:.2em} @@ -1028,7 +1027,7 @@ In that example PUGIXML_API is inconsistent between several source

PUGIXML_HAS_LONG_LONG define enables support for long long type in pugixml. This define is automatically enabled if your platform is known to have long long support (i.e. has C++11 support or uses a reasonably modern version of a known compiler); if pugixml does not recognize that your platform supports long long but in fact it does, you can enable the define manually.

-

PUGIXML_HAS_STRING_VIEW define enables function overloads that accept std::basic_string_view arguments. This define is automatically enabled if built targeting c++17 or later AND if PUGIXML_STRING_VIEW is also defined. The requirement to additionally define PUGIXML_STRING_VIEW will be retired in a future version.

+

PUGIXML_HAS_STRING_VIEW define enables function overloads that accept std::string_view arguments. This define is automatically enabled if built targeting C++17 or later; if pugixml does not recognize that your platform supports std::string_view but in fact it does, you can enable the define manually.

@@ -1295,7 +1294,7 @@ If the size of wchar_t is 2, pugixml assumes UTF-16 encoding instea

-There is a special type, pugi::char_t, that is defined as the character type and depends on the library configuration; it will be also used in the documentation hereafter. There is also a type pugi::string_t, which is defined as the STL string of the character type; it corresponds to std::string in char mode and to std::wstring in wchar_t mode. Similarly, string_view_t is defined to be std::basic_string_view<char_t>. Overloads for string_view_t are only available when building for c++17 or later (see PUGIXML_HAS_STRING_VIEW).

+There is a special type, pugi::char_t, that is defined as the character type and depends on the library configuration; it will be also used in the documentation hereafter. There is also a type pugi::string_t, which is defined as the STL string of the character type; it corresponds to std::string in char mode and to std::wstring in wchar_t mode. Similarly, string_view_t is defined to be std::basic_string_view<char_t>. Overloads for string_view_t are only available when building for C++17 or later (see PUGIXML_HAS_STRING_VIEW).

In addition to the interface, the internal implementation changes to store XML data as pugi::char_t; this means that these two modes have different memory usage characteristics - generally UTF-8 mode is more memory and performance efficient, especially if sizeof(wchar_t) is 4. The conversion to pugi::char_t upon document loading and from pugi::char_t upon document saving happen automatically, which also carries minor performance penalty. The general advice however is to select the character mode based on usage scenario, i.e. if UTF-8 is inconvenient to process and most of your XML data is non-ASCII, wchar_t mode is probably a better choice.

@@ -6217,7 +6216,7 @@ If exceptions are disabled, then in the event of parsing failure the query is in
From af6cbeb17004c140ad3cfe3dec8174790138e496 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 30 Oct 2024 11:25:17 -0700 Subject: [PATCH 3/5] Enable C++17 standard in AppVeyor tests To avoid increasing the build matrix we enable this unconditionally for VS2019 and VS2022 to be able to test std::string_view. Note that we already test VS2022 without this flag on GHA so this should catch any regressions. --- tests/autotest-appveyor.ps1 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/autotest-appveyor.ps1 b/tests/autotest-appveyor.ps1 index 26d8e81..68d39be 100644 --- a/tests/autotest-appveyor.ps1 +++ b/tests/autotest-appveyor.ps1 @@ -37,6 +37,8 @@ foreach ($vs in $args) if (! $?) { throw "Error setting up VS$vs $arch" } + $cxx = if ($vs -ge 19) { "/std:c++17" } else { "" } + foreach ($defines in "standard", "PUGIXML_WCHAR_MODE", "PUGIXML_COMPACT") { $target = "tests_vs${vs}_${arch}_${defines}" @@ -45,7 +47,7 @@ foreach ($vs in $args) Add-AppveyorTest $target -Outcome Running Write-Output "# Building $target.exe" - & cmd /c "cl.exe /MP /Fe$target.exe /EHsc /W4 /WX $deflist $sources 2>&1" | Tee-Object -Variable buildOutput + & cmd /c "cl.exe /MP /Fe$target.exe /EHsc /W4 /WX $cxx $deflist $sources 2>&1" | Tee-Object -Variable buildOutput if ($?) { From ae163d5f0617c6cc5a91957961ed4d669a16b4b1 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 30 Oct 2024 11:46:36 -0700 Subject: [PATCH 4/5] Enable C++17 support in VS2019 and VS2022 projects This enables std::string_view support for NuGet builds --- scripts/pugixml_vs2019.vcxproj | 4 ++++ scripts/pugixml_vs2019_static.vcxproj | 4 ++++ scripts/pugixml_vs2022.vcxproj | 4 ++++ scripts/pugixml_vs2022_static.vcxproj | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/scripts/pugixml_vs2019.vcxproj b/scripts/pugixml_vs2019.vcxproj index 690a790..464229c 100644 --- a/scripts/pugixml_vs2019.vcxproj +++ b/scripts/pugixml_vs2019.vcxproj @@ -99,6 +99,7 @@ $(IntDir)$(TargetName).pdb OldStyle false + stdcpp17 Windows @@ -115,6 +116,7 @@ $(IntDir)$(TargetName).pdb OldStyle false + stdcpp17 Windows @@ -132,6 +134,7 @@ WIN32;NDEBUG;_LIB;%(PreprocessorDefinitions) $(IntDir)$(TargetName).pdb OldStyle + stdcpp17 Windows @@ -151,6 +154,7 @@ NDEBUG;_LIB;%(PreprocessorDefinitions) $(IntDir)$(TargetName).pdb OldStyle + stdcpp17 Windows diff --git a/scripts/pugixml_vs2019_static.vcxproj b/scripts/pugixml_vs2019_static.vcxproj index 1747e37..04fa09c 100644 --- a/scripts/pugixml_vs2019_static.vcxproj +++ b/scripts/pugixml_vs2019_static.vcxproj @@ -100,6 +100,7 @@ OldStyle false MultiThreadedDebug + stdcpp17 Windows @@ -117,6 +118,7 @@ OldStyle false MultiThreadedDebug + stdcpp17 Windows @@ -135,6 +137,7 @@ $(IntDir)$(TargetName).pdb OldStyle MultiThreaded + stdcpp17 Windows @@ -155,6 +158,7 @@ $(IntDir)$(TargetName).pdb OldStyle MultiThreaded + stdcpp17 Windows diff --git a/scripts/pugixml_vs2022.vcxproj b/scripts/pugixml_vs2022.vcxproj index efb1350..ee8e224 100644 --- a/scripts/pugixml_vs2022.vcxproj +++ b/scripts/pugixml_vs2022.vcxproj @@ -99,6 +99,7 @@ $(IntDir)$(TargetName).pdb OldStyle false + stdcpp17 Windows @@ -115,6 +116,7 @@ $(IntDir)$(TargetName).pdb OldStyle false + stdcpp17 Windows @@ -132,6 +134,7 @@ WIN32;NDEBUG;_LIB;%(PreprocessorDefinitions) $(IntDir)$(TargetName).pdb OldStyle + stdcpp17 Windows @@ -151,6 +154,7 @@ NDEBUG;_LIB;%(PreprocessorDefinitions) $(IntDir)$(TargetName).pdb OldStyle + stdcpp17 Windows diff --git a/scripts/pugixml_vs2022_static.vcxproj b/scripts/pugixml_vs2022_static.vcxproj index 065a9dc..6dd0956 100644 --- a/scripts/pugixml_vs2022_static.vcxproj +++ b/scripts/pugixml_vs2022_static.vcxproj @@ -100,6 +100,7 @@ OldStyle false MultiThreadedDebug + stdcpp17 Windows @@ -117,6 +118,7 @@ OldStyle false MultiThreadedDebug + stdcpp17 Windows @@ -135,6 +137,7 @@ $(IntDir)$(TargetName).pdb OldStyle MultiThreaded + stdcpp17 Windows @@ -155,6 +158,7 @@ $(IntDir)$(TargetName).pdb OldStyle MultiThreaded + stdcpp17 Windows From 0f22f71f60b44f2047c3d4385b309b02edf50f14 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 30 Oct 2024 12:02:30 -0700 Subject: [PATCH 5/5] CMake now uses C++17 if supported by the compiler We only set this when C++ version or requirement flag is not overridden externally to be able to rely on CMake automatically downgrading the standard version when the compiler doesn't support it. CXX_STANDARD 17 also requires CMake 3.8 or later; on earlier versions we use the old behavior and set C++11. --- CMakeLists.txt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cf43abf..6542658 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -64,11 +64,13 @@ if (NOT DEFINED CMAKE_MSVC_RUNTIME_LIBRARY) MultiThreaded$<$:Debug>$<$>:DLL>) endif() -if (NOT DEFINED CMAKE_CXX_STANDARD_REQUIRED) - set(CMAKE_CXX_STANDARD_REQUIRED ON) -endif() +# Set the default C++ standard to C++17 if not set; CMake will automatically downgrade this if the compiler does not support it +# When CMAKE_CXX_STANDARD_REQUIRED is set, we fall back to C++11 to avoid breaking older compilers +if (NOT DEFINED CMAKE_CXX_STANDARD_REQUIRED AND NOT DEFINED CMAKE_CXX_STANDARD AND NOT CMAKE_VERSION VERSION_LESS 3.8) -if (NOT DEFINED CMAKE_CXX_STANDARD) + set(CMAKE_CXX_STANDARD 17) + set(CMAKE_CXX_STANDARD_REQUIRED OFF) +elseif (NOT DEFINED CMAKE_CXX_STANDARD) set(CMAKE_CXX_STANDARD 11) endif()