Skip to content

Remove dead MSVC version fallbacks in CGAL_GeneratorSpecificSettings.cmake#9394

Open
RajdeepKushwaha5 wants to merge 1 commit intoCGAL:mainfrom
RajdeepKushwaha5:fix/remove-dead-msvc-fallbacks-generator-settings
Open

Remove dead MSVC version fallbacks in CGAL_GeneratorSpecificSettings.cmake#9394
RajdeepKushwaha5 wants to merge 1 commit intoCGAL:mainfrom
RajdeepKushwaha5:fix/remove-dead-msvc-fallbacks-generator-settings

Conversation

@RajdeepKushwaha5
Copy link
Contributor

Summary of Changes

Remove the dead elseif(MSVC14) through elseif(MSVC71) fallback chain in Installation/cmake/modules/CGAL_GeneratorSpecificSettings.cmake.

These fallbacks targeted Visual Studio 2003 (VC 7.1) through Visual Studio 2015 (VC14). They are unreachable dead code because:

  1. CGAL requires CMake >= 3.15, and MSVC_TOOLSET_VERSION has been defined for all MSVC versions since CMake 3.12. The if(MSVC_TOOLSET_VERSION) branch always fires for any MSVC build, making the elseif chain unreachable.

  2. CGAL requires C++17, which means Visual Studio 2017 (15.3) at minimum. None of the removed fallback versions (VS 2003–2015) can compile CGAL.

The else() branch (non-MSVC compilers) is preserved as elseif(NOT MSVC) to maintain the status message for GCC/Clang/etc.

Before:

if ( MSVC_TOOLSET_VERSION )
  set(CGAL_TOOLSET "vc${MSVC_TOOLSET_VERSION}")
  message( STATUS "Using VC toolset ${MSVC_TOOLSET_VERSION}." )
elseif ( MSVC14 )
  ...  # 7 dead elseif branches (VC14, VC12, VC11, VC10, VC90, VC80, VC71)
else()
  message( STATUS "Using ${CMAKE_CXX_COMPILER} compiler." )
endif()

After:

if ( MSVC_TOOLSET_VERSION )
  set(CGAL_TOOLSET "vc${MSVC_TOOLSET_VERSION}")
  message( STATUS "Using VC toolset ${MSVC_TOOLSET_VERSION}." )
elseif ( NOT MSVC )
  message( STATUS "Using ${CMAKE_CXX_COMPILER} compiler." )
endif()

Release Management

Copilot AI review requested due to automatic review settings March 21, 2026 14:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies CGAL’s generator/toolset detection logic by removing legacy MSVC version fallback branches that are no longer relevant given CGAL’s current CMake/C++ requirements.

Changes:

  • Removed the elseif(MSVC14)elseif(MSVC71) fallback chain from CGAL_GeneratorSpecificSettings.cmake.
  • Kept non-MSVC status reporting by changing the final branch to elseif(NOT MSVC).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

set(CGAL_TOOLSET "vc71")
message( STATUS "Using VC71 compiler." )
else()
elseif ( NOT MSVC )
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, if MSVC is true but MSVC_TOOLSET_VERSION is not set/empty, the script now produces no status output and leaves CGAL_TOOLSET unset (previously it would at least hit the else() message). If the intent is to rely on cmake_minimum_required(VERSION 3.15...) guaranteeing MSVC_TOOLSET_VERSION, consider adding an explicit elseif(MSVC) branch that errors or emits a clear diagnostic so unexpected toolchains/CMake versions fail loudly instead of silently skipping compiler/toolset reporting.

Suggested change
elseif ( NOT MSVC )
elseif ( MSVC )
message(AUTHOR_WARNING
"MSVC is detected but MSVC_TOOLSET_VERSION is not set. "
"This may indicate an unsupported CMake version or toolchain; "
"compiler/toolset reporting and CGAL_TOOLSET configuration may be incomplete.")
else()

Copilot uses AI. Check for mistakes.
…cmake

Remove the dead elseif(MSVC14) through elseif(MSVC71) fallback chain.
These are unreachable because CGAL requires CMake >= 3.15, which always
defines MSVC_TOOLSET_VERSION for any MSVC build. Additionally, CGAL
requires C++17, which needs VS 2017+ at minimum.

Add an explicit elseif(MSVC) branch with AUTHOR_WARNING for the edge
case where MSVC is detected but MSVC_TOOLSET_VERSION is unexpectedly
empty, so it fails loudly instead of silently skipping toolset config.

The else() branch preserves the status message for non-MSVC compilers.
@RajdeepKushwaha5 RajdeepKushwaha5 force-pushed the fix/remove-dead-msvc-fallbacks-generator-settings branch from a26ec2e to 7be47bb Compare March 21, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants