Skip to content

build(cmake): Force include CppMacros.h globally for VC6#2252

Open
abhatem wants to merge 1 commit intoTheSuperHackers:mainfrom
abhatem:main
Open

build(cmake): Force include CppMacros.h globally for VC6#2252
abhatem wants to merge 1 commit intoTheSuperHackers:mainfrom
abhatem:main

Conversation

@abhatem
Copy link

@abhatem abhatem commented Feb 3, 2026

Many source files in Generals and GeneralsMD use modern C++ features like 'nullptr' (polyfilled in CppMacros.h) without explicitly including the header. This caused build failures on VC6 (Fixes #2165).

This change:

  1. Adds 'Dependencies/Utility' to the global include path (guarded by IS_VS6_BUILD).
  2. Uses the MSVC '/FI' flag to force include 'Utility/CppMacros.h' when compiling with VC6. This ensures compatibility macros like 'nullptr' (defined as 0) are always available.

@greptile-apps
Copy link

greptile-apps bot commented Feb 3, 2026

Greptile Overview

Greptile Summary

This PR fixes VC6 build failures by forcing inclusion of CppMacros.h, which provides C++11 compatibility polyfills like nullptr (defined as 0 for older compilers). Many source files use these modern features without explicitly including the header.

  • Added Dependencies/Utility to global include path for VC6 builds
  • Used MSVC /FI flag to force include Utility/CppMacros.h in all compilation units
  • Changes are properly guarded with IS_VS6_BUILD condition
  • Includes comment indicating this is temporary until VC6 support is dropped

The implementation is clean, follows established patterns in the codebase, and solves the build issue without requiring modifications to hundreds of source files.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change uses standard MSVC compiler flags, is properly guarded by IS_VS6_BUILD conditional, and follows established patterns in the codebase. The forced include approach is a well-known workaround that doesn't affect other build configurations.
  • No files require special attention

Important Files Changed

Filename Overview
CMakeLists.txt Added forced include of CppMacros.h for VC6 builds to provide C++11 polyfills like nullptr

Sequence Diagram

sequenceDiagram
    participant CMake
    participant Compiler as MSVC VC6
    participant Sources as Source Files
    participant CppMacros as CppMacros.h
    
    CMake->>CMake: Check IS_VS6_BUILD
    alt IS_VS6_BUILD is TRUE
        CMake->>CMake: Add Dependencies/Utility to include paths
        CMake->>Compiler: Set /FIUtility/CppMacros.h flag
        loop For each source file
            Compiler->>CppMacros: Force include at start of compilation
            CppMacros->>Compiler: Define nullptr as 0
            CppMacros->>Compiler: Define other C++11 polyfills
            Compiler->>Sources: Compile with polyfills available
        end
    else IS_VS6_BUILD is FALSE
        Compiler->>Sources: Compile normally (C++11 support)
    end
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@Skyaero42 Skyaero42 added the Build Anything related to building, compiling label Feb 4, 2026
Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Ok interesting. Thanks for fixing.

CMakeLists.txt Outdated

# To be removed when abandoning VC6
if (IS_VS6_BUILD)
include_directories(${CMAKE_SOURCE_DIR}/Dependencies/Utility)
Copy link

Choose a reason for hiding this comment

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

Can we omit ${CMAKE_SOURCE_DIR} or is that strictly required?

Copy link
Author

Choose a reason for hiding this comment

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

true, it's not needed, it's a remnant from when I was experimenting 😅

Also after giving it a second look today, I didn't like the generator expression I had: it had a check for if it's a MSVC compiler ($<CXX_COMPILER_ID:MSVC>:) while it's enclosed in a if (IS_VS6_BUILD) and cmake generators are ugly imo. I rewrote it to a more readable line now.

@xezon xezon added Major Severity: Minor < Major < Critical < Blocker CompileBug Bug at compile time labels Feb 4, 2026
Many source files in Generals and GeneralsMD use modern C++ features like 'nullptr' (polyfilled in CppMacros.h) without explicitly including the header. This caused build failures on VC6 (Fixes TheSuperHackers#2165).

This change:
1. Adds 'Dependencies/Utility' to the global include path (guarded by IS_VS6_BUILD).
2. Uses the MSVC '/FI' flag to force include 'Utility/CppMacros.h'
   when compiling with VC6. This ensures compatibility macros like
   'nullptr' (defined as 0) are always available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build Anything related to building, compiling CompileBug Bug at compile time Major Severity: Minor < Major < Critical < Blocker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nullptr undeclared identifier when compiling using docker on github codespaces

3 participants