Skip to content
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

End-to-end tagging: C++ #8316

Merged
merged 9 commits into from
Dec 9, 2024
Merged

End-to-end tagging: C++ #8316

merged 9 commits into from
Dec 9, 2024

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Dec 4, 2024

Implemented with the help of @Wumpf.

This semantically mimics very closely the way things are done in Rust, minus all technical differences due to the differences between both the languages and the SDKs.
For that reason, everything stated in #8304 (comment) basically applies as-is.

Pretty happy about it, I must say.

@teh-cmc teh-cmc added enhancement New feature or request do-not-merge Do not merge this PR 🌊 C++ API C/C++ API specific 🔩 data model labels Dec 4, 2024
Copy link

github-actions bot commented Dec 4, 2024

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

  • I have tested the web viewer
Result Commit Link
87ec140 https://rerun.io/viewer/pr/8316

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

@teh-cmc teh-cmc force-pushed the cmc/end_to_end_tags_rust branch from a20cd08 to f201277 Compare December 4, 2024 14:41
@teh-cmc teh-cmc force-pushed the cmc/end_to_end_tags_cpp branch from 087fe42 to 74dd688 Compare December 4, 2024 14:44
@teh-cmc teh-cmc marked this pull request as ready for review December 4, 2024 14:44
@teh-cmc teh-cmc requested a review from Wumpf December 4, 2024 14:44
Comment on lines +55 to +139
/// Unconditionally sets `archetype_name` to the given one.
ComponentDescriptor with_archetype_name(std::optional<std::string_view> archetype_name_
) const {
ComponentDescriptor descriptor = *this;
descriptor.archetype_name = archetype_name_;
return descriptor;
}

/// Unconditionally sets `archetype_name` to the given one.
ComponentDescriptor with_archetype_name(const char* archetype_name_) const {
ComponentDescriptor descriptor = *this;
descriptor.archetype_name = archetype_name_;
return descriptor;
}

/// Unconditionally sets `archetype_field_name` to the given one.
ComponentDescriptor with_archetype_field_name(
std::optional<std::string_view> archetype_field_name_
) const {
ComponentDescriptor descriptor = *this;
descriptor.archetype_field_name = archetype_field_name_;
return descriptor;
}

/// Unconditionally sets `archetype_field_name` to the given one.
ComponentDescriptor with_archetype_field_name(const char* archetype_field_name_) const {
ComponentDescriptor descriptor = *this;
descriptor.archetype_field_name = archetype_field_name_;
return descriptor;
}

/// Sets `archetype_name` to the given one iff it's not already set.
ComponentDescriptor or_with_archetype_name(std::optional<std::string_view> archetype_name_
) const {
if (this->archetype_field_name.has_value()) {
return *this;
}
ComponentDescriptor descriptor = *this;
descriptor.archetype_name = archetype_name_;
return descriptor;
}

/// Sets `archetype_name` to the given one iff it's not already set.
ComponentDescriptor or_with_archetype_name(const char* archetype_name_) const {
if (this->archetype_field_name.has_value()) {
return *this;
}
ComponentDescriptor descriptor = *this;
descriptor.archetype_name = archetype_name_;
return descriptor;
}

/// Sets `archetype_field_name` to the given one iff it's not already set.
ComponentDescriptor or_with_archetype_field_name(
std::optional<std::string_view> archetype_field_name_
) const {
if (this->archetype_field_name.has_value()) {
return *this;
}
ComponentDescriptor descriptor = *this;
descriptor.archetype_field_name = archetype_field_name_;
return descriptor;
}

/// Sets `archetype_field_name` to the given one iff it's not already set.
ComponentDescriptor or_with_archetype_field_name(const char* archetype_field_name_) const {
if (this->archetype_field_name.has_value()) {
return *this;
}
ComponentDescriptor descriptor = *this;
descriptor.archetype_field_name = archetype_field_name_;
return descriptor;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I tried to not repeat myself so much, but anytime I try to do something any fancier than that, either clang and/or gcc complains or things just segfault. Whatever.

@teh-cmc teh-cmc force-pushed the cmc/end_to_end_tags_rust branch from a82a5bb to 82a1dae Compare December 4, 2024 14:49
@teh-cmc teh-cmc force-pushed the cmc/end_to_end_tags_cpp branch from 74dd688 to 470cbd3 Compare December 4, 2024 14:49
@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 4, 2024

@rerun-bot full-check

Copy link

github-actions bot commented Dec 4, 2024

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

@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 4, 2024

TODO: end-to-end failed on arm64 mac:

 FAILED: docs/snippets/CMakeFiles/descr_custom_archetype.dir/all/descriptors/descr_custom_archetype.cpp.o 
/Users/runner/work/rerun/rerun/.pixi/envs/cpp/bin/x86_64-apple-darwin13.4.0-clang++  -I/Users/runner/work/rerun/rerun/rerun_cpp/src -march=core2 -mtune=haswell -mssse3 -ftree-vectorize -fPIC -fstack-protector-strong -O2 -pipe -stdlib=libc++ -fvisibility-inlines-hidden -fmessage-length=0 -isystem /Users/runner/work/rerun/rerun/.pixi/envs/cpp/include -g -isysroot /Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -Wall -Wcast-align -Wcast-qual -Wextra -Wformat=2 -Wmissing-include-dirs -Wnull-dereference -Wold-style-cast -Wpedantic -Wpointer-arith -Wshadow -Wswitch-enum -Wunreachable-code -Wvla -Wc++17-compat-pedantic -Wc++20-compat-pedantic -Wc99-extensions -Weverything -Wgnu -Wnon-gcc -Wpre-c2x-compat-pedantic -Wshadow-all -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-covered-switch-default -Wno-ctad-maybe-unsupported -Wno-disabled-macro-expansion -Wno-documentation -Wno-documentation-unknown-command -Wno-double-promotion -Wno-exit-time-destructors -Wno-float-equal -Wno-global-constructors -Wno-missing-prototypes -Wno-padded -Wno-reserved-id-macro -Wno-reserved-identifier -Wno-unreachable-code-break -Wno-unreachable-code-return -Wno-unused-macros -Wno-unsafe-buffer-usage -Wno-unknown-warning-option -Werror -g -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fno-optimize-sibling-calls -fsanitize=address -Werror -MD -MT docs/snippets/CMakeFiles/descr_custom_archetype.dir/all/descriptors/descr_custom_archetype.cpp.o -MF docs/snippets/CMakeFiles/descr_custom_archetype.dir/all/descriptors/descr_custom_archetype.cpp.o.d -o docs/snippets/CMakeFiles/descr_custom_archetype.dir/all/descriptors/descr_custom_archetype.cpp.o -c /Users/runner/work/rerun/rerun/docs/snippets/all/descriptors/descr_custom_archetype.cpp
/Users/runner/work/rerun/rerun/docs/snippets/all/descriptors/descr_custom_archetype.cpp:68:38: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
    CustomPosition3D positions[1] = {rerun::components::Position3D{1.0f, 2.0f, 3.0f}};
                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                     {                                              }
1 error generated.
[597/617] Building CXX object docs/snippets/CMakeFiles/descr_builtin_component.dir/all/descriptors/descr_builtin_component.cpp.o
[598/617] Building CXX object docs/snippets/CMakeFiles/different_data_per_timeline.dir/all/concepts/different_data_per_timeline.cpp.o
[599/617] Building CXX object docs/snippets/CMakeFiles/descr_builtin_archetype.dir/all/descriptors/descr_builtin_archetype.cpp.o
[600/617] Linking CXX executable docs/snippets/native-async
[601/617] Building CXX object docs/snippets/CMakeFiles/descr_custom_component.dir/all/descriptors/descr_custom_component.cpp.o
FAILED: docs/snippets/CMakeFiles/descr_custom_component.dir/all/descriptors/descr_custom_component.cpp.o 
/Users/runner/work/rerun/rerun/.pixi/envs/cpp/bin/x86_64-apple-darwin13.4.0-clang++  -I/Users/runner/work/rerun/rerun/rerun_cpp/src -march=core2 -mtune=haswell -mssse3 -ftree-vectorize -fPIC -fstack-protector-strong -O2 -pipe -stdlib=libc++ -fvisibility-inlines-hidden -fmessage-length=0 -isystem /Users/runner/work/rerun/rerun/.pixi/envs/cpp/include -g -isysroot /Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -Wall -Wcast-align -Wcast-qual -Wextra -Wformat=2 -Wmissing-include-dirs -Wnull-dereference -Wold-style-cast -Wpedantic -Wpointer-arith -Wshadow -Wswitch-enum -Wunreachable-code -Wvla -Wc++17-compat-pedantic -Wc++20-compat-pedantic -Wc99-extensions -Weverything -Wgnu -Wnon-gcc -Wpre-c2x-compat-pedantic -Wshadow-all -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-covered-switch-default -Wno-ctad-maybe-unsupported -Wno-disabled-macro-expansion -Wno-documentation -Wno-documentation-unknown-command -Wno-double-promotion -Wno-exit-time-destructors -Wno-float-equal -Wno-global-constructors -Wno-missing-prototypes -Wno-padded -Wno-reserved-id-macro -Wno-reserved-identifier -Wno-unreachable-code-break -Wno-unreachable-code-return -Wno-unused-macros -Wno-unsafe-buffer-usage -Wno-unknown-warning-option -Werror -g -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fno-optimize-sibling-calls -fsanitize=address -Werror -MD -MT docs/snippets/CMakeFiles/descr_custom_component.dir/all/descriptors/descr_custom_component.cpp.o -MF docs/snippets/CMakeFiles/descr_custom_component.dir/all/descriptors/descr_custom_component.cpp.o.d -o docs/snippets/CMakeFiles/descr_custom_component.dir/all/descriptors/descr_custom_component.cpp.o -c /Users/runner/work/rerun/rerun/docs/snippets/all/descriptors/descr_custom_component.cpp
/Users/runner/work/rerun/rerun/docs/snippets/all/descriptors/descr_custom_component.cpp:32:38: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
    CustomPosition3D positions[1] = {rerun::components::Position3D{1.0f, 2.0f, 3.0f}};
                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                     {                                              }
1 error generated.

@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 5, 2024

@rerun-bot full-check

Copy link

github-actions bot commented Dec 5, 2024

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

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

looking great!
Didn't understand though how indicator components work now in this world - did you just change the interim definitions of indicator components to not contain the archetype name?

crates/build/re_types_builder/src/codegen/cpp/mod.rs Outdated Show resolved Hide resolved
@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 5, 2024

Didn't understand though how indicator components work now in this world - did you just change the interim definitions of indicator components to not contain the archetype name?

For now, indicators just set their component_name (same is true of all languages, so that roundtrips pass).

This will change soon when we enter milestone 2: #8129 (comment), but one issue at a time.

@teh-cmc teh-cmc force-pushed the cmc/end_to_end_tags_rust branch from 82a1dae to deb1615 Compare December 5, 2024 15:04
@teh-cmc teh-cmc force-pushed the cmc/end_to_end_tags_cpp branch from e9be108 to 98a76e4 Compare December 5, 2024 15:04
@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 5, 2024

@rerun-bot full-check

Copy link

github-actions bot commented Dec 5, 2024

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

@teh-cmc teh-cmc force-pushed the cmc/end_to_end_tags_cpp branch from 90ef75b to 18cb6b3 Compare December 6, 2024 10:27
@rerun-io rerun-io deleted a comment from github-actions bot Dec 6, 2024
@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 6, 2024

oh god what now 😂

@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 6, 2024

@rerun-bot full-check

@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 6, 2024

this is the one 👇

Copy link

github-actions bot commented Dec 6, 2024

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

@teh-cmc teh-cmc force-pushed the cmc/end_to_end_tags_rust branch 2 times, most recently from d8abb0b to 772fb23 Compare December 9, 2024 09:11
Base automatically changed from cmc/end_to_end_tags_rust to main December 9, 2024 09:25
@teh-cmc teh-cmc force-pushed the cmc/end_to_end_tags_cpp branch from 7dd6190 to 87ec140 Compare December 9, 2024 09:27
@teh-cmc teh-cmc removed the do-not-merge Do not merge this PR label Dec 9, 2024
@teh-cmc teh-cmc merged commit 962f3e0 into main Dec 9, 2024
34 of 35 checks passed
@teh-cmc teh-cmc deleted the cmc/end_to_end_tags_cpp branch December 9, 2024 09:32
teh-cmc added a commit that referenced this pull request Dec 9, 2024
This semantically mimics very closely the way things are done in Rust,
minus all technical differences due to the differences between both the
languages and the SDKs.
For that reason, everything stated in
#8304 (comment) basically
applies as-is.

Pretty happy about it, I must say.

* DNM: requires #8316 
* Part of #7948
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants