Skip to content

Fix MSVC C++20 compilation issues #9951

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented May 10, 2025

Related

What

Removes default arguments from from_loggable since they caused static assertions to trigger during overload resolve
already. This is technically a breaking change in the interface but I wouldn't feel bad about bending semvar and shipping it in a 0.23.3 release!

Added C++20 MSVC test run to CI. All C++20 tests run now on main/nightly only, not on every pr (starts getting too much)

  • ci changes confirmed working

Removes default arguments from `from_loggable` since they caused static assertions to trigger during overload resolve already.
@Wumpf Wumpf added 🪳 bug Something isn't working 🪟 windows Problems specific to the Windows OS 🌊 C++ API C/C++ API specific include in changelog labels May 10, 2025
@Wumpf Wumpf force-pushed the andreas/fix-cpp20-windows-issues branch 2 times, most recently from 50c59c2 to e813b79 Compare May 10, 2025 20:54
@Wumpf Wumpf added this to the 0.23.3 (?) milestone May 10, 2025
@Wumpf Wumpf force-pushed the andreas/fix-cpp20-windows-issues branch from e813b79 to 84d21f3 Compare May 10, 2025 20:55
Copy link

github-actions bot commented May 10, 2025

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
ef0ff10 https://rerun.io/viewer/pr/9951 +nightly +main

Note: This comment is updated whenever you push a commit.

@Wumpf Wumpf force-pushed the andreas/fix-cpp20-windows-issues branch from 84d21f3 to c012aa5 Compare May 10, 2025 20:57
@Wumpf
Copy link
Member Author

Wumpf commented May 10, 2025

@rerun-bot full-check

Copy link

@emilk
Copy link
Member

emilk commented May 11, 2025

@rerun-bot full-check

Copy link

Started a full build: https://github.com/rerun-io/rerun/actions/runs/14954315744

@emilk
Copy link
Member

emilk commented May 11, 2025

CI failure is also there on main

@Wumpf Wumpf added the do-not-merge Do not merge this PR label May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working 🌊 C++ API C/C++ API specific do-not-merge Do not merge this PR include in changelog 🪟 windows Problems specific to the Windows OS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants