yhirose / cpp-peglib

A single file C++ header-only PEG (Parsing Expression Grammars) library
MIT License
916 stars 113 forks source link

Compiling with CMake under MSVC results in "Requires complete C++17 support" #307

Closed Raikiri closed 1 month ago

Raikiri commented 1 month ago

In my CMake I tried adding

set(CMAKE_CXX_STANDARD 17)

as well as

set_property(TARGET target PROPERTY CXX_STANDARD 20)

But it results in: cpp-peglib\peglib.h(41,1): error C1189: #error: "Requires complete C++17 support"

I also tried copying that line that I saw in peglib's CMake:

if(MSVC)
  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /Zc:__cplusplus")
endif()

But that also didn't work...

Raikiri commented 1 month ago

Related issue: https://gitlab.kitware.com/cmake/cmake/-/issues/18837

yhirose commented 1 month ago

@Raikiri than you for the report. I don't understand this issue, since I have no problem with my Visual Studio and GitHub Actions CI on Windows (windows-latest, windows-2019). https://github.com/yhirose/cpp-peglib/actions/runs/10689158008

Could you make sure that you are using recent Visual Studio that officially supports C++17?

Raikiri commented 1 month ago

Ah, I figured out the issue. I was doing this:

if(MSVC)
  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /Zc:__cplusplus /utf-8")
endif()

but I was doing this before calling a project() function. Apparently the MSVC is not defined at that point and that condition never triggered.

So yeah. On one hand, it's my fault for not knowing this neat quirk of cmake. On the other hand, CMake as usual is being CMake. On the third hand though, why is that even needed? MSVC does support C++17, it just for whatever reason can't define the __cplusplus macro, that for whatever reason CMake also fails to define by default. So maybe just fix this in the source instead?

#if !((defined(__cplusplus) && __cplusplus >= 201703L) || (defined(_MSC_VER) && _MSC_VER >= 1910))
#error "Requires complete C++17 support"
#endif
yhirose commented 1 month ago

@Raikiri thanks for the report, and I am glad that you found the solution.

Currently, cpp-peglib is totally vender free code, and just depends on the C++17 standard. So I would like to avoid introducing any vender specific macro such as _MSC_VER in peglib.h.

Sorry for that, and thanks for your understanding.