Cxx23 side quest #68
Cxx23 side quest #68
Conversation
- Wrap bedrock_smoke_step test in BUILD_WITH_OCCT guard - Test requires OCCT-dependent WriteTSEasSTEP function - Test is now only built when BEDROCK_WITH_OPENCASCADE=ON - Prevents linker errors when OCCT is not available (default on macOS) Full preset build (macos-arm64-debug) now completes without errors.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
CMakeLists.txt
Outdated
|
|
||
| # Side Quest 1: Simple Job (C++23 vs C++20 comparison) | ||
| add_executable(bedrock_simple_job experiments/simple_job.cpp) | ||
| target_link_libraries(bedrock_simple_job PRIVATE bedrock_engine) |
There was a problem hiding this comment.
Bug: ODR violation when OCCT is enabled
The bedrock_simple_job target links to bedrock_engine, which conditionally links to bedrock_geom when OCCT is enabled. The file experiments/simple_job.cpp defines a stub implementation of bedrock::geom::WriteTSEasSTEP. When BEDROCK_WITH_OCCT=ON, both bedrock_geom (with the real implementation) and simple_job.cpp (with the stub) would be linked, causing a One Definition Rule violation and linker errors from multiple definitions of the same symbol. The stub should be conditionally compiled only when OCCT is disabled.
| runs-on: ${{ matrix.os }} | ||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
Bug: Matrix jobs share concurrency group, will cancel each other
The tests job is a matrix build with os: [ubuntu-24.04, macos-14], but the concurrency group is ${{ github.workflow }}-${{ github.ref }} which doesn't include ${{ matrix.os }}. This means both matrix variants share the same concurrency group. With cancel-in-progress: true, when the second matrix job starts, it will cancel the first one. Only one OS variant will complete instead of both. The concurrency group should include the matrix variable, e.g., ${{ github.workflow }}-${{ github.ref }}-${{ matrix.os }}.
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| # submodules: recursive | ||
| ======= | ||
| uses: actions/checkout@v6 | ||
| >>>>>>> origin/main |
There was a problem hiding this comment.
Bug: Unresolved git merge conflict markers in CI workflow
The CI workflow file contains unresolved git merge conflict markers (<<<<<<< HEAD, =======, >>>>>>> origin/main) at two locations. These markers break YAML syntax and will cause GitHub Actions to fail when parsing the workflow file. The first conflict is between actions/checkout@v4 and actions/checkout@v6, and the second conflict involves the Build step versus Upload/cleanup steps. These conflicts need to be resolved before merging.
Side Quest 1: Bedrock C++23 Feasibility & CI Modernization
1. Executive Summary
This PR concludes the C++23 Feasibility Side Quest.
The goal was to prove that Bedrock can be compiled, linked, and tested under C++23 across all supported platforms and CI environments.
Result: Fully Feasible.
docs/SQ1_CPP23_FEASIBILITY.mddetails the toolchain matrix and probe results.2. Major Changes
Core Infrastructure
bedrocktoCMAKE_CXX_STANDARD 23.bedrock/experiments/cpp23_probe.cppto verify critical features (std::expected,std::ranges,if consteval).bedrock/experiments/simple_job.cppto verify engine execution and ABI stability.CI Modernization (.github/workflows/ci.yml)
ubuntu-24.04(GCC 13) andmacos-14(Apple Clang 15).--modulesargument for Qt 6.10.1aqt.venv-basedaqtinstall to bypass PEP 668 restrictions on system Python.install-qt-actiondependency with robustCMAKE_PREFIX_PATHderivation directly from theaqtinstall layout.Documentation
docs/SQ1_CPP23_FEASIBILITY.mdtracking the entire validation process.3. What’s NOT Included
4. Definition of Done
main.5. Reviewer Instructions
cmake -B build -DCMAKE_CXX_STANDARD=23on your machine to confirm local hygiene.Note
Hardens Palantir with input validation/standardized errors and size limits, bumps Bedrock to 0.3.0, adds extensive unit/integration tests and coverage, introduces C++23 probe targets and CMake presets, modernizes CI (Ubuntu 24.04/macOS) to build+ctest, and lands comprehensive docs updates.
ErrorResponse, and 10MBMAX_MESSAGE_SIZEchecks inPalantirServer; updateCapabilitiesserver_versiontobedrock-0.3.0.BEDROCK_WITH_OPENCASCADE(alias legacyBEDROCK_WITH_OCCT) and conditional OCCT linking; bump project to0.3.0.CMakePresets.json; introduce experimental targetscpp23_probeandbedrock_simple_job(guarded by OCCT state).ctest; add coverage (Linux) and artifact uploads; introduce a full original CI spec..gitignorePhoenix sub-build ignores added.Written by Cursor Bugbot for commit f072813. This will update automatically on new commits. Configure here.