diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 741c4267..b723bfe9 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -152,37 +152,6 @@ }, "selectedYankedVersions": {}, "moduleExtensions": { - "@@apple_support+//crosstool:setup.bzl%apple_cc_configure_extension": { - "general": { - "bzlTransitiveDigest": "E970FlMbwpgJPdPUQzatKh6BMfeE0ZpWABvwshh7Tmg=", - "usagesDigest": "aYRVMk+1OupIp+5hdBlpzT36qgd6ntgSxYTzMLW5K4U=", - "recordedFileInputs": {}, - "recordedDirentsInputs": {}, - "envVariables": {}, - "generatedRepoSpecs": { - "local_config_apple_cc_toolchains": { - "repoRuleId": "@@apple_support+//crosstool:setup.bzl%_apple_cc_autoconf_toolchains", - "attributes": {} - }, - "local_config_apple_cc": { - "repoRuleId": "@@apple_support+//crosstool:setup.bzl%_apple_cc_autoconf", - "attributes": {} - } - }, - "recordedRepoMappingEntries": [ - [ - "apple_support+", - "bazel_tools", - "bazel_tools" - ], - [ - "bazel_tools", - "rules_cc", - "rules_cc+" - ] - ] - } - }, "@@rules_fuzzing+//fuzzing/private:extensions.bzl%non_module_dependencies": { "general": { "bzlTransitiveDigest": "mGiTB79hRNjmeDTQdzkpCHyzXhErMbufeAmySBt7s5s=", @@ -268,7 +237,7 @@ }, "@@rules_kotlin+//src/main/starlark/core/repositories:bzlmod_setup.bzl%rules_kotlin_extensions": { "general": { - "bzlTransitiveDigest": "hUTp2w+RUVdL7ma5esCXZJAFnX7vLbVfLd7FwnQI6bU=", + "bzlTransitiveDigest": "OlvsB0HsvxbR8ZN+J9Vf00X/+WVz/Y/5Xrq2LgcVfdo=", "usagesDigest": "QI2z8ZUR+mqtbwsf2fLqYdJAkPOHdOV+tF2yVAUgRzw=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, diff --git a/exploration/BUILD.bazel b/exploration/BUILD.bazel index 37ae3622..9f9b5b0d 100644 --- a/exploration/BUILD.bazel +++ b/exploration/BUILD.bazel @@ -33,3 +33,20 @@ build_test( name = "traits_and_concepts_build_test", targets = ["traits_and_concepts"], ) + +cc_library( + name = "recursive_variant", + srcs = ["recursive_variant.cc"], + hdrs = ["recursive_variant.h"], + visibility = ["//visibility:private"], + deps = ["//:indirect"], +) + +cc_test( + name = "recursive_variant_test", + srcs = ["recursive_variant_test.cc"], + deps = [ + "recursive_variant", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/exploration/CMakeLists.txt b/exploration/CMakeLists.txt index 7e09808a..ae42b060 100644 --- a/exploration/CMakeLists.txt +++ b/exploration/CMakeLists.txt @@ -8,6 +8,15 @@ if (${XYZ_VALUE_TYPES_IS_NOT_SUBPROJECT}) incomplete_types_test.cc ) + xyz_add_test( + NAME recursive_variant_test + LINK_LIBRARIES indirect + FILES + recursive_variant.h + recursive_variant.cc + recursive_variant_test.cc + ) + add_library(traits_and_concepts OBJECT) target_sources(traits_and_concepts PRIVATE diff --git a/exploration/incomplete_types.h b/exploration/incomplete_types.h index 2f961e3d..aaf24279 100644 --- a/exploration/incomplete_types.h +++ b/exploration/incomplete_types.h @@ -1,5 +1,8 @@ // Wrapper types with members templated on an incomplete type. +#ifndef XYZ_EXPLORATION_INCOMPLETE_TYPES_H_ +#define XYZ_EXPLORATION_INCOMPLETE_TYPES_H_ + #include #include #include @@ -70,3 +73,5 @@ struct VariantVectorMember { bool operator==(const VariantVectorMember& other) const; }; } // namespace xyz::testing + +#endif // XYZ_EXPLORATION_INCOMPLETE_TYPES_H_ diff --git a/exploration/recursive_variant.cc b/exploration/recursive_variant.cc new file mode 100644 index 00000000..d7f929be --- /dev/null +++ b/exploration/recursive_variant.cc @@ -0,0 +1 @@ +#include "recursive_variant.h" diff --git a/exploration/recursive_variant.h b/exploration/recursive_variant.h new file mode 100644 index 00000000..50996b1a --- /dev/null +++ b/exploration/recursive_variant.h @@ -0,0 +1,34 @@ +// An investigation of recursive variants and an associated helper type. + +#ifndef XYZ_EXPLORATION_RECURSIVE_VARIANT_H_ +#define XYZ_EXPLORATION_RECURSIVE_VARIANT_H_ + +#include +#include + +#include "indirect.h" + +namespace xyz::testing { + +template +struct deref { + T t_; + + using U = decltype(*std::declval()); + + operator U&() { return *t_; } + + operator const U&() const { return *t_; } +}; + +struct ASTNode; +using ASTNodeRecursiveStorage = deref>; +using ASTNodeData = std::variant; + +struct ASTNode { + ASTNodeData data_; +}; + +} // namespace xyz::testing + +#endif // XYZ_EXPLORATION_RECURSIVE_VARIANT_H_ diff --git a/exploration/recursive_variant_test.cc b/exploration/recursive_variant_test.cc new file mode 100644 index 00000000..09de4bda --- /dev/null +++ b/exploration/recursive_variant_test.cc @@ -0,0 +1,55 @@ +#include "recursive_variant.h" + +#include + +namespace { + +using xyz::testing::ASTNode; +using xyz::testing::ASTNodeRecursiveStorage; +using xyz::testing::deref; + +template +struct overload : Ts... { + using Ts::operator()...; + + overload(Ts&&... ts) : Ts(std::forward(ts))... {} +}; + +TEST(RecursiveVariant, ExplicitAccess) { + ASTNode node; + + // This is a pain to write and exposes implementation details. + int result = + std::visit(overload([](const int&) { return 0; }, // + [](const std::string&) { return 1; }, // + [](const ASTNodeRecursiveStorage&) { return 2; }), + node.data_); + + EXPECT_EQ(result, 0); +} + +TEST(RecursiveVariant, DerefAccess) { + ASTNode node; + + // This is nicer to write. + int result = std::visit(overload([](const int&) { return 0; }, // + [](const std::string&) { return 1; }, // + [](const ASTNode&) { return 2; }), + node.data_); + + EXPECT_EQ(result, 0); +} + +TEST(RecursiveVariant, LazyAccess) { + ASTNode node; + + // This is lazy. + int result = std::visit(overload([](const int&) { return 0; }, // + [](const std::string&) { return 1; }, // + [](const auto&) { return 2; }), + node.data_); + + EXPECT_EQ(result, 0); +} + +} // namespace diff --git a/proposals/DRAFT_NO_IMPLICIT_CONVERSIONS.md b/proposals/DRAFT_NO_IMPLICIT_CONVERSIONS.md index 4a5a8330..6dda958b 100644 --- a/proposals/DRAFT_NO_IMPLICIT_CONVERSIONS.md +++ b/proposals/DRAFT_NO_IMPLICIT_CONVERSIONS.md @@ -8,7 +8,7 @@ D3902R2 Working Group: Library Evolution -Date: 2025-11-3 +Date: 2025-11-6 _Jonathan Coe \<\>_ @@ -105,6 +105,10 @@ J. B. Coe, A. Peacock, and S. Parent, 2023 \ ## History +### Changes in R2 + +Add Appendix discussing recursive variants. + ### Changes in R1 * Add clarification that `operator->` and `operator*` (including const-qualified and @@ -113,3 +117,154 @@ associated `indirect` instance(s) must not be in a valueless state. * Add clarification that the authors would like to see implementation and **usage** experience to motivate the introduction of a `reference_wrapper`-like API. + +## Appendix: Recursive variants and `OneOf` + +Taken from the National Body Comment author's variant library (OneOf) there is a type `indirection`, +similar to `indirect` and reproduced below. + +`indirection` has a reference-wrapper-like API but has much more scope for undefined behaviour than +`indirect` as accepted into the C++26 working draft. + +Copy construction, move construction, assignment and move assignment of `indirection` all require that +`other` has not been moved-from. `operator T&` and `get` (const and non-const qualified) all require that +`this` has not been moved from. As designed, `indirection` gives a user no way to check that an instance +of `indirection` has not been moved from. + + + +``` +template +using disable_capturing = + std::enable_if_t>::value, + int>; + +template +struct indirection { + public: + indirection() : p_(new T{}) {} + + indirection(indirection&& other) noexcept : p_(other.p_) { + other.p_ = nullptr; + } + + indirection& operator=(indirection&& other) noexcept { + this->~indirection(); + return *::new ((void*)this) indirection{std::move(other)}; + } + + indirection(indirection const& other) : indirection(other.get()) {} + + indirection& operator=(indirection const& other) { + if (p_) { + get() = other.get(); + } else { + indirection tmp{other}; + swap(*this, tmp); + } + + return *this; + } + + template = 0> + explicit indirection(A&& a, As&&... as) + : p_(new T(std::forward(a), std::forward(as)...)) {} + + ~indirection() { delete p_; } + + friend void swap(indirection& x, indirection& y) noexcept { + std::swap(x.p_, y.p_); + } + + operator T&() { return this->get(); } + + operator T const&() const { return this->get(); } + + T& get() { return *p_; } + + T const& get() const { return *p_; } + + private: + T* p_; +}; +``` + +The variant in OneOf uses `indirection` to implement recursive variants. + +A recursive variant can be implemented with `indirect` but `indirect`'s absence +of an implicit conversion to a reference type means that a pointer-like interface is +exposed to users. + +Improvements to the usability of recursive variants could be made by introducing another type to automatically dereference indirect storage. We illustrate the use of a helper type below. + +Any design to better support recursive variants would be best addressed with a concrete proposal +and the authors of indirect invite the author of the National Body Comment to put forward a paper +for the C++29 standard. + +``` +template +struct deref { + T t_; + + using U = decltype(*std::declval()); + + operator U&() { return *t_; } + + operator const U&() const { return *t_; } +}; + +struct ASTNode; +using ASTNodeRecursiveStorage = deref>; +using ASTNodeData = std::variant; + +struct ASTNode { + ASTNodeData data_; +}; + +/// Access tests. + +template +struct overload : Ts... { + using Ts::operator()...; + + overload(Ts&&... ts) : Ts(std::forward(ts))... {} +}; + +TEST(RecursiveVariant, ExplicitAccess) { + ASTNode node; + + // This is a pain to write and exposes implementation details. + int result = + std::visit(overload([](const int&) { return 0; }, // + [](const std::string&) { return 1; }, // + [](const ASTNodeRecursiveStorage&) { return 2; }), + node.data_); + + EXPECT_EQ(result, 0); +} + +TEST(RecursiveVariant, DerefAccess) { + ASTNode node; + + // This is nicer to write. + int result = std::visit(overload([](const int&) { return 0; }, // + [](const std::string&) { return 1; }, // + [](const ASTNode&) { return 2; }), + node.data_); + + EXPECT_EQ(result, 0); +} + +TEST(RecursiveVariant, LazyAccess) { + ASTNode node; + + // This is lazy. + int result = std::visit(overload([](const int&) { return 0; }, // + [](const std::string&) { return 1; }, // + [](const auto&) { return 2; }), + node.data_); + + EXPECT_EQ(result, 0); +} + +```