Skip to content

Windows compatibility (new PR) - #526

Open
andrewxu319 wants to merge 30 commits into
developfrom
andrew/windows-compatibility-v2
Open

Windows compatibility (new PR)#526
andrewxu319 wants to merge 30 commits into
developfrom
andrew/windows-compatibility-v2

Conversation

@andrewxu319

Copy link
Copy Markdown
Collaborator

Description

A continuation of #445. I started a new branch/PR because of rebasing issues in the old one. I've also addressed/responded to all comments on the old PR.

Please describe the issue that is addressed (bug, new feature,
documentation, enhancement, etc.). Please also include relevant motivation and
context. List any dependencies that are required for this change.

Make compiler-specific changes such that GridKit runs and pass all tests on Windows. Specifically, it now works with MinGW G++, LLVM/Clang, and MSVC.

Closes #(issue)

Mentions @(user)

Proposed changes

Describe how your changes here address the issue and why the proposed changes
should be accepted.

  • Append .string() to .filename() to extract the underlying string if the OS is Windows
  • Replace __attribute__((always_inline)) inline with a compiler-specific FORCE_INLINE macro, defined in root CMakeLists.txt (it seems to have been added in an earlier commit then removed. I added it back)
  • Inline Utilities::toUpper() and Utilities::strip to avoid ODR-related errors
  • Update examples/Consumer/CMakeLists.txt and create a PowerShell script so that test_install works on Windows

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here
to help! This is simply a reminder of what we are going to look for before
merging your code.

All tests pass and make test_install works on all three compilers. On MSVC, use cmake --build . --target test_install instead of make test_install.

  • All tests pass.
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows GridKit™ style guidelines.
  • [N/A] There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch.
  • I have updated CHANGELOG.md to reflect the changes in this PR. If this is a minor PR that is part of a larger fix already included in the file, state so.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining
why you chose the solution you did and what alternatives you considered, etc...

@pelesh pelesh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good. I suggest we merge it after 0.2 release.

Comment thread GridKit/Definitions.hpp.in Outdated
@andrewxu319

Copy link
Copy Markdown
Collaborator Author

Looks good. I suggest we merge it after 0.2 release.

Keeping this as a draft since I found more issues with the G++ build. I'll undraft it after fixing those.

@pelesh pelesh added this to the Release 0.3 milestone Aug 7, 2026
@pelesh pelesh added enhancement New feature or request development Features/Tools related to development of GridKit, rather than use as a library. cmake labels Aug 7, 2026
@andrewxu319
andrewxu319 marked this pull request as ready for review August 7, 2026 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmake development Features/Tools related to development of GridKit, rather than use as a library. enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants