biojppm / rapidyaml

Rapid YAML - a library to parse and emit YAML, and do it fast.
MIT License
568 stars 97 forks source link

single header: missing include of charconv #445

Closed linuslau closed 3 months ago

linuslau commented 3 months ago

When attempting to compile with C++17, I encountered an error: error C2039: 'chars_format': is not a member of 'std'. This issue arises because 'chars_format' was introduced in C++17. However, the necessary

include is conditionally included in charconv.hpp as follows:

#if __cplusplus >= 201703L
#include <charconv>
#endif

The macro cplusplus does not meet the condition, so the #include is not added, causing the above error. Investigating the value of cplusplus, I found it is always set to 199711L, regardless of whether the C++ standard is specified as 11 or 17. This suggests potential issues even with C++11.

Given this, I've added the /Zc:cplusplus compiler option for MSVC to ensure the cplusplus macro is set correctly. This change was tested in an environment using VS2022, where MSVC does not set the __cplusplus macro correctly even when the C++ version is specified.

biojppm commented 3 months ago

This is surprising.

C++17 is tested with vs2019+vs2022 quite extensively in the CI; see eg https://github.com/biojppm/rapidyaml/actions/runs/9619886934 .

Can you provide a minimum verifiable example of the problem? What are the exact commands (including any cmake options/compile defines) you are doing in your config+compilation that cause the error you are seeing? And what is the exact error you are seeing?

linuslau commented 3 months ago

Output.txt

On my end, the issue can be reproduced whether using the command line, VS Code, or VS2022. I will provide the simplest reproduction process using the command line.

There are two typical types:

Here are the infoamtion of two methods respectively.

Reproduction process for the first method:

git clone --recursive https://github.com/biojppm/rapidyaml
cd rapidyaml
make code change as "Change 1" below
cmake -B out
cmake --build out

Will get __cplusplus in MSVC = 199711L in the "Output 1" from attached log Output.txt, suppose 201103L should be expected number, not sure this has any side effect to the project.

Change 1 (to print __cplusplus):

c:\Users\kz\Documents\Code\YAML\rapidyaml>git diff src/c4/yml/tree.cpp
diff --git a/src/c4/yml/tree.cpp b/src/c4/yml/tree.cpp
index a5fe9c6..2e97006 100644
--- a/src/c4/yml/tree.cpp
+++ b/src/c4/yml/tree.cpp
@@ -9,10 +9,13 @@ C4_SUPPRESS_WARNING_GCC_CLANG_WITH_PUSH("-Wold-style-cast")
 C4_SUPPRESS_WARNING_GCC("-Wtype-limits")
 C4_SUPPRESS_WARNING_GCC("-Wuseless-cast")

+#define STRINGIZE(x) #x
+#define TOSTRING(x) STRINGIZE(x)
+#pragma message("__cplusplus in MSVC = " TOSTRING(__cplusplus))
+
 namespace c4 {

Reproduction process for the second method

git clone --recursive https://github.com/biojppm/rapidyaml
cd rapidyaml/samples/singleheaderlib
make code change as "Change 2" below
cmake -B out
cmake --build out

Will get multiple error in "Output 2" in attached Output.txt, I didn't copy them all, as all related to one issue mentioned.

Change 2 (keep anyone of the method to enable 17 or all of them there):

diff --git a/samples/singleheaderlib/CMakeLists.txt b/samples/singleheaderlib/CMakeLists.txt
index 9b0d387..c7e583e 100644
--- a/samples/singleheaderlib/CMakeLists.txt
+++ b/samples/singleheaderlib/CMakeLists.txt
@@ -1,19 +1,23 @@
 cmake_minimum_required(VERSION 3.15 FATAL_ERROR)
 project(ryml-quickstart LANGUAGES CXX)

+set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD_REQUIRED True)
+
 #create a target to amalgamate ryml into a single header:
 include(../singleheader/amalgamate.cmake)
 amalgamate_ryml(SINGLE_HEADER_DIR SINGLE_HEADER)

 #add a library using the amalgamated header
 add_library(ryml lib.cpp ${SINGLE_HEADER})
-target_compile_features(ryml PUBLIC cxx_std_11)
+target_compile_features(ryml PUBLIC cxx_std_17)
 target_include_directories(ryml PUBLIC "${SINGLE_HEADER_DIR}")

 #now simply define the executable:
 add_executable(ryml-quickstart ../quickstart.cpp)
 target_link_libraries(ryml-quickstart PRIVATE ryml)
 target_compile_definitions(ryml-quickstart PUBLIC -DRYML_SINGLE_HEADER_LIB)
+target_compile_features(ryml-quickstart PUBLIC cxx_std_17)

 #adjustments for shared library
 if(BUILD_SHARED_LIBS)

diff --git a/samples/singleheaderlib/lib.cpp b/samples/singleheaderlib/lib.cpp
index aa33e6e..5e7b8c4 100644
--- a/samples/singleheaderlib/lib.cpp
+++ b/samples/singleheaderlib/lib.cpp
@@ -1,2 +1,5 @@
 #define RYML_SINGLE_HDR_DEFINE_NOW
+#define STRINGIZE(x) #x
+#define TOSTRING(x) STRINGIZE(x)
+#pragma message("__cplusplus in MSVC = " TOSTRING(__cplusplus))
 #include <ryml_all.hpp>
linuslau commented 3 months ago

Accidentally closed the issue, reopened again.

biojppm commented 3 months ago

I was able to reproduce using the following command (no changes required):

set -xe ; rm -rf src_singleheader ; cd samples/singleheaderlib ; cmake -B build -D CMAKE_CXX_STANDARD=17 ; cmake --build build -j

This is caused by a problem in the single-header amalgamation.

biojppm commented 3 months ago

Thanks for reporting!