Bump cmake version, fix clang-cl #99

Febbe · 2025-09-24T21:30:16Z

Fixes #98

Croydon

There are a lot of changes that are unrelated to increase the minimum CMake version, and many of those changes are non-obvious, that require much more explanation of their motivation and effects IMHO.

Croydon · 2025-09-25T13:00:31Z

CMakeLists.txt

@@ -1,69 +1,58 @@
-cmake_minimum_required(VERSION 3.0.0 FATAL_ERROR)
+cmake_minimum_required(VERSION 3.21.0)
+if(POLICY CMP0177)


This is CMake 3.31+ only and could unexpectely change behaviour based on CMake version https://cmake.org/cmake/help/v3.31/policy/CMP0177.html

Seems unnecessary

The thing is, we already have CMake 4.x and CMake doesn't support this POLICY at some point. So it issues many warnings. It seems to work fine with the new policy if available.

I can test other ways to silence those warnings. Maybe setting that POLICY explicitly to OLD.

Croydon · 2025-09-25T13:01:37Z

CMakeLists.txt


 if(NOT MSVC)
-   message( WARNING "Visual Studio Build supported only" )
+   message(FATAL_ERROR "Visual Studio Build supported only" )


This might break exotic configurations that actually work, for basically no advantage.

Ok, will remove it.

Croydon · 2025-09-25T13:04:34Z

buildClangCl.bat

@@ -0,0 +1,5 @@
+mkdir CMakeBuildClangCl


I think we should stop adding more of those bat files. In times where CMake is well integrated in editors, we really shouldn't need them. There are highly unflexible anyway

Ok, I'll remove it, shall I remove the others too?

Just my make my role here clear: I contributed to winflexbison before, but I have no official say here in any way. So I am stating my own personal opinions here.

I am not sure if someone still relies on the existing bat files, but since the development of winflexbison is mostly dead, it can't affect too many people for sure, if we would remove all of them. But maybe this is something for another pull request.

Another pull request it is... concerning cmake I'll let @Croydon decide, for the rest... stay compatible :-)

Croydon · 2025-09-25T13:05:24Z

bison/src/config.h


 extern int strverscmp(const char* s1, const char* s2);

+struct obstack;


Why does a CMake version increase require a code change like this?

Not the cmake bump itself, but that code boils out with clang-cl.

The declaration of struct obstack in the functions parameter list is not visible to the outside. It is basically static.

clang is more strict and interprets it as a different declaration each time this file is included and linked. Therefore we get ODR violations on link time.

Croydon · 2025-09-25T13:06:41Z

common/m4/lib/clean-temp.h

+#define	_MODE_T_
 typedef int mode_t;
-#endif
+#endif /* _MODE_T_ */


Seems reasonable, although unrelated.

It's not unrelated, since it's required to work with clang-cl.
Clang-cl uses vcruntime and MSVC-STL so no mode_t is defined, but clang obviously defines clang.

Sorry, my bad. I was too focused on CMake version adaptations, so I forgot that you want to fix clang-cl midway.

Croydon · 2025-09-25T13:07:26Z

flex/CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.0.0 FATAL_ERROR)
+cmake_minimum_required(VERSION 3.20.0)


Why do you use 3.20.0 here, if you propose 3.21.0 for the parent project and bison?

Croydon · 2025-09-25T13:08:00Z

CMakeLists.txt

+  add_compile_definitions("__extension__=")
+endif()

+#if (MSVC)


Those comments can easily be interpreted as actual code and are confusing.

Croydon · 2025-09-25T13:08:19Z

CMakeLists.txt

+    set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE "${CMAKE_CURRENT_BINARY_DIR}/bin/Release")
+    set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELWITHDEBINFO "${CMAKE_CURRENT_BINARY_DIR}/bin/RelWithDebInfo")
+endif()
+    # Only apply to MSVC frontend (not clang frontends)


Spacing is off

Febbe · 2025-09-26T10:46:04Z

CMakeLists.txt

+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /W3 /EHsc")
+set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} /GL /Oi /Gy")


changed this from CXX to C, since all projects are C projects. Those settings had zero effect before.
Shall I remove those lines (behavior unchanged) or use the new flags (lot of warnings, most likely unused intended behavior).

Bump cmake version, fix clang-cl

0c2cf15

Croydon reviewed Sep 25, 2025

View reviewed changes

Febbe commented Sep 26, 2025

View reviewed changes


		extern int strverscmp(const char* s1, const char* s2);

		struct obstack;

		@@ -1,4 +1,4 @@
		cmake_minimum_required(VERSION 3.0.0 FATAL_ERROR)
		cmake_minimum_required(VERSION 3.20.0)

		set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /W3 /EHsc")
		set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} /GL /Oi /Gy")

Bump cmake version, fix clang-cl #99

Are you sure you want to change the base?

Bump cmake version, fix clang-cl #99

Uh oh!

Conversation

Febbe commented Sep 24, 2025

Uh oh!

Croydon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants