Skip to content

Commit 5727915

Browse files
Wumpfemilk
andauthored
Fix MSVC C++20 compilation issues (#9951)
Co-authored-by: Emil Ernerfeldt <[email protected]>
1 parent 54f9013 commit 5727915

File tree

5 files changed

+50
-21
lines changed

5 files changed

+50
-21
lines changed

.github/workflows/cpp_matrix_full.json

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,36 @@
55
"runs_on": "ubuntu-latest-16-cores",
66
"cache_key": "build-linux",
77
"extra_env_vars": "RERUN_USE_ASAN=1 RERUN_SET_CXX_VERSION=17 LSAN_OPTIONS=suppressions=.github/workflows/lsan_suppressions.supp",
8-
"additional_commands": "pixi run -e cpp cpp-docs"
8+
"additional_commands": "pixi run -e cpp cpp-docs",
9+
"pr_ci": true
910
},
1011
{
11-
"name": "Linux x64, C++20",
12+
"name": "Linux x64, C++20 (skipped on PRs)",
1213
"runs_on": "ubuntu-latest-16-cores",
1314
"cache_key": "build-linux",
14-
"extra_env_vars": "RERUN_USE_ASAN=1 RERUN_SET_CXX_VERSION=20 LSAN_OPTIONS=suppressions=.github/workflows/lsan_suppressions.supp"
15+
"extra_env_vars": "RERUN_USE_ASAN=1 RERUN_SET_CXX_VERSION=20 LSAN_OPTIONS=suppressions=.github/workflows/lsan_suppressions.supp",
16+
"pr_ci": false
1517
},
1618
{
17-
"name": "Windows x64",
19+
"name": "Windows x64, C++17",
1820
"runs_on": "windows-latest-8-cores",
1921
"cache_key": "build-windows",
20-
"extra_env_vars": ""
22+
"extra_env_vars": "RERUN_SET_CXX_VERSION=17",
23+
"pr_ci": true
24+
},
25+
{
26+
"name": "Windows x64, C++20 (skipped on PRs)",
27+
"runs_on": "windows-latest-8-cores",
28+
"cache_key": "build-windows",
29+
"extra_env_vars": "RERUN_SET_CXX_VERSION=20",
30+
"pr_ci": false
2131
},
2232
{
2333
"name": "Mac aarch64",
2434
"runs_on": "macos-15-large",
2535
"cache_key": "build-macos-arm64",
26-
"extra_env_vars": "RERUN_USE_ASAN=1 LSAN_OPTIONS=suppressions=.github/workflows/lsan_suppressions.supp"
36+
"extra_env_vars": "RERUN_USE_ASAN=1 LSAN_OPTIONS=suppressions=.github/workflows/lsan_suppressions.supp",
37+
"pr_ci": true
2738
}
2839
]
2940
}

.github/workflows/reusable_checks_cpp.yml

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,17 +62,23 @@ jobs:
6262
matrix: ${{ fromJson(needs.matrix_prep.outputs.MATRIX) }}
6363
runs-on: ${{ matrix.runs_on }}
6464
steps:
65+
# Skipping the entire step would apparently require a separate job, not doing that here.
66+
# Instead we keep checking for the `matrix.pr_ci` flag.
67+
# See https://stackoverflow.com/questions/77186893/how-can-i-skip-the-whole-job-for-a-matrix-match-in-github-action
6568
- uses: actions/checkout@v4
69+
if: ${{ github.event_name != 'pull_request' || matrix.pr_ci != false }}
6670
with:
6771
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || '' }}
6872

6973
- uses: prefix-dev/[email protected]
74+
if: ${{ github.event_name != 'pull_request' || matrix.pr_ci != false }}
7075
with:
7176
pixi-version: v0.41.4
7277
environments: cpp
7378

7479
- name: Set up Rust
7580
uses: ./.github/actions/setup-rust
81+
if: ${{ github.event_name != 'pull_request' || matrix.pr_ci != false }}
7682
with:
7783
cache_key: ${{ matrix.cache_key }}
7884
# Cache will be produced by `reusable_checks/rs-lints`
@@ -82,26 +88,30 @@ jobs:
8288

8389
# Workaround for ASAN issues on Github images https://github.com/actions/runner-images/issues/9491
8490
- name: Fix kernel mmap rnd bits
85-
if: runner.os == 'Linux'
91+
if: ${{ (github.event_name != 'pull_request' || matrix.pr_ci != false) && runner.os == 'Linux' }}
8692
# Asan in llvm 14 provided in ubuntu 22.04 is incompatible with
8793
# high-entropy ASLR in much newer kernels that GitHub runners are
8894
# using leading to random crashes: https://reviews.llvm.org/D148280
8995
run: sudo sysctl vm.mmap_rnd_bits=28
9096

9197
- name: pixi run -e cpp cpp-clean
98+
if: ${{ github.event_name != 'pull_request' || matrix.pr_ci != false }}
9299
run: pixi run -e cpp cpp-clean
93100

94101
- name: pixi run -e cpp cpp-build-all
102+
if: ${{ github.event_name != 'pull_request' || matrix.pr_ci != false }}
95103
run: ${{ matrix.extra_env_vars }} RERUN_WERROR=ON pixi run -e cpp cpp-build-all
96104

97105
- name: pixi run -e cpp cpp-test
106+
if: ${{ github.event_name != 'pull_request' || matrix.pr_ci != false }}
98107
run: ${{ matrix.extra_env_vars }} RERUN_WERROR=ON pixi run -e cpp cpp-test
99108

100109
- name: pixi run -e cpp cpp-build-all-shared-libs
101110
if: ${{ inputs.CHANNEL == 'nightly' }}
102111
run: ${{ matrix.extra_env_vars }} RERUN_WERROR=ON pixi run -e cpp cpp-build-all-shared-libs
103112

104113
- name: additional_commands
114+
if: ${{ github.event_name != 'pull_request' || matrix.pr_ci != false }}
105115
run: ${{ matrix.additional_commands }}
106116

107117
cpp-formatting:

docs/snippets/all/descriptors/descr_builtin_component.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ int main() {
66

77
rec.log_static(
88
"data",
9-
rerun::ComponentBatch::from_loggable(rerun::Position3D(1.0f, 2.0f, 3.0f))
9+
rerun::ComponentBatch::from_loggable(
10+
rerun::Position3D(1.0f, 2.0f, 3.0f),
11+
rerun::Loggable<rerun::Position3D>::Descriptor
12+
)
1013
);
1114

1215
// The tags are indirectly checked by the Rust version (have a look over there for more info).

rerun_cpp/src/rerun/component_batch.hpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@ namespace rerun {
4444
/// Automatically registers the descriptor the first time it is encountered.
4545
template <typename T>
4646
static Result<ComponentBatch> from_loggable(
47-
const rerun::Collection<T>& components,
48-
const ComponentDescriptor& descriptor = rerun::Loggable<T>::Descriptor
47+
const rerun::Collection<T>& components, const ComponentDescriptor& descriptor
4948
) {
5049
static_assert(
5150
rerun::is_loggable<T>,
@@ -64,8 +63,7 @@ namespace rerun {
6463
/// Automatically registers the descriptor the first time it is encountered.
6564
template <typename T>
6665
static Result<ComponentBatch> from_loggable(
67-
const T& component,
68-
const ComponentDescriptor& descriptor = rerun::Loggable<T>::Descriptor
66+
const T& component, const ComponentDescriptor& descriptor
6967
) {
7068
// Collection adapter will automatically borrow for single elements, but let's do this explicitly, avoiding the extra hoop.
7169
const auto collection = Collection<T>::borrow(&component, 1);
@@ -79,8 +77,7 @@ namespace rerun {
7977
/// Automatically registers the descriptor the first time it is encountered.
8078
template <typename T>
8179
static Result<ComponentBatch> from_loggable(
82-
const std::optional<T>& component,
83-
const ComponentDescriptor& descriptor = rerun::Loggable<T>::Descriptor
80+
const std::optional<T>& component, const ComponentDescriptor& descriptor
8481
) {
8582
if (component.has_value()) {
8683
return from_loggable(component.value(), descriptor);
@@ -97,7 +94,7 @@ namespace rerun {
9794
template <typename T>
9895
static Result<ComponentBatch> from_loggable(
9996
const std::optional<rerun::Collection<T>>& components,
100-
const ComponentDescriptor& descriptor = rerun::Loggable<T>::Descriptor
97+
const ComponentDescriptor& descriptor
10198
) {
10299
if (components.has_value()) {
103100
return from_loggable(components.value(), descriptor);

rerun_cpp/tests/recording_stream.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,13 @@ SCENARIO("RecordingStream can be used for logging archetypes and components", TE
148148

149149
GIVEN("component batches") {
150150
auto batch0 = rerun::ComponentBatch::from_loggable<rerun::Position2D>(
151-
{{1.0, 2.0}, {4.0, 5.0}}
152-
).value_or_throw();
151+
{{1.0, 2.0}, {4.0, 5.0}},
152+
rerun::Loggable<rerun::Position2D>::Descriptor
153+
)
154+
.value_or_throw();
153155
auto batch1 = rerun::ComponentBatch::from_loggable<rerun::Color>(
154-
{rerun::Color(0xFF0000FF)}
156+
{rerun::Color(0xFF0000FF)},
157+
rerun::Loggable<rerun::Color>::Descriptor
155158
)
156159
.value_or_throw();
157160
THEN("single component batch can be logged") {
@@ -170,10 +173,12 @@ SCENARIO("RecordingStream can be used for logging archetypes and components", TE
170173
}
171174
GIVEN("component batches wrapped in `rerun::Result`") {
172175
auto batch0 = rerun::ComponentBatch::from_loggable<rerun::Position2D>(
173-
{{1.0, 2.0}, {4.0, 5.0}}
176+
{{1.0, 2.0}, {4.0, 5.0}},
177+
rerun::Loggable<rerun::Position2D>::Descriptor
174178
);
175179
auto batch1 = rerun::ComponentBatch::from_loggable<rerun::Color>(
176-
{rerun::Color(0xFF0000FF)}
180+
{rerun::Color(0xFF0000FF)},
181+
rerun::Loggable<rerun::Color>::Descriptor
177182
);
178183
THEN("single component batch can be logged") {
179184
stream.log("log_archetype-splat", batch0);
@@ -406,7 +411,10 @@ SCENARIO("Recording stream handles serialization failure during logging graceful
406411
expected_error.code =
407412
GENERATE(rerun::ErrorCode::Unknown, rerun::ErrorCode::ArrowStatusCode_TypeError);
408413

409-
auto batch_result = rerun::ComponentBatch::from_loggable(component);
414+
auto batch_result = rerun::ComponentBatch::from_loggable(
415+
component,
416+
rerun::Loggable<BadComponent>::Descriptor
417+
);
410418

411419
THEN("calling log with that batch logs the serialization error") {
412420
check_logged_error([&] { stream.log(path, batch_result); }, expected_error.code);

0 commit comments

Comments
 (0)