Skip to content

Conversation

eramongodb
Copy link
Contributor

Related to CXX-2745. A minor PR as followup to #1447 to improve the v_noabi -> v1 migration path for renamed entities by providing (and using) equivalent aliases in the current v_noabi API.

Every rename is accompanied by:

  • the new name in v_noabi whose behavior is identical to the renamed v_noabi entity, and
  • a deprecation note in the old name pointing to the new name to be used instead.

This permits users to easily switch to the renamed v_noabi equivalent without needing to deal with v_noabi vs. v1 relative breaking changes. Users who have switched to the renamed v_noabi equivalent will then be able to manually (via ABI namespace qualification) or automatically (via root namespace redeclaration updates) switch to the v1 equivalent. For example, the migration path for v_noabi::types::bson_value::view -> v1::types::view may look like:

// Current (v_noabi):
bsoncxx::types::bson_value::view v; // v_noabi
v.get_owning_value();

// Renamed (v_noabi):
bsoncxx::types::view v; // v_noabi
v.type_value();

// Root Namespace Redeclaration Update (v_noabi -> v1):
bsoncxx::types::view v; // v1
v.type_value();

Without the renames introduced by this PR, the middle step is missing: users would have to jump straight from the old v_noabi name to the new v1 equivalent.

The renamed (deprecated) names may be removed in an upcoming API major release by moving the entity definitions into where the aliases/equivalents are currently declared or by dropping them entirely in favor of v1. Either approach will work depending on the timeline of events (does not need to be decided at this time).

@eramongodb eramongodb self-assigned this Sep 17, 2025
@eramongodb eramongodb requested a review from a team as a code owner September 17, 2025 14:45
@eramongodb
Copy link
Contributor Author

Relating to CXX-2118 (view vs. value API symmetry), overlooked an old recurring issue with declaring a .foo() member function given an existing foo namespace-scope declaration with GCC compilers (this is not an issue with Clang or MSVC compilers):

In file included from bsoncxx/types/bson_value/value.cpp:15:
bsoncxx/types/bson_value/value.hpp:307:19: error: declaration of ‘bsoncxx::v_noabi::type bsoncxx::v_noabi::types::bson_value::value::type() const’ changes meaning of ‘type’ [-fpermissive]
  307 |     v_noabi::type type() const {
      |                   ^~~~
In file included from bsoncxx/types/bson_value/view.hpp:31,
                 from bsoncxx/types/bson_value/value.hpp:34,
                 from bsoncxx/types/bson_value/value.cpp:15:
bsoncxx/types.hpp:47:12: note: ‘type’ declared here as ‘enum class bsoncxx::v_noabi::type’
   47 | enum class type : std::uint8_t {
      |            ^~~~

It remains a mystery to me why v_noabi::types::bson_value::view does not seem to trigger this same error despite declaring type() const in the same manner as v_noabi::types::bson_value::value would.

@connorsmacd
Copy link
Contributor

connorsmacd commented Sep 17, 2025

Relating to CXX-2118 (view vs. value API symmetry), overlooked an old recurring issue with declaring a .foo() member function given an existing foo namespace-scope declaration with GCC compilers (this is not an issue with Clang or MSVC compilers):

In file included from bsoncxx/types/bson_value/value.cpp:15:
bsoncxx/types/bson_value/value.hpp:307:19: error: declaration of ‘bsoncxx::v_noabi::type bsoncxx::v_noabi::types::bson_value::value::type() const’ changes meaning of ‘type’ [-fpermissive]
  307 |     v_noabi::type type() const {
      |                   ^~~~
In file included from bsoncxx/types/bson_value/view.hpp:31,
                 from bsoncxx/types/bson_value/value.hpp:34,
                 from bsoncxx/types/bson_value/value.cpp:15:
bsoncxx/types.hpp:47:12: note: ‘type’ declared here as ‘enum class bsoncxx::v_noabi::type’
   47 | enum class type : std::uint8_t {
      |            ^~~~

It remains a mystery to me why v_noabi::types::bson_value::view does not seem to trigger this same error despite declaring type() const in the same manner as v_noabi::types::bson_value::value would.

Does changing the declaration to:

enum v_noabi::type type() const;

make the error go away? I couldn't repro your exact issue, but this works when the error is expected, e.g.: https://godbolt.org/z/xbhj39cfW

@eramongodb
Copy link
Contributor Author

eramongodb commented Sep 17, 2025

Solved: it's apparently being caused by this stray unqualified type in a constructor parameter list (?!): https://godbolt.org/z/YnWb9nzKE

Removing the ctor declaration or qualifying the declaration with v_noabi resolves the GCC compilation error. I'm quite suspicious now that this may be a GCC compiler bug. 😵

@connorsmacd
Copy link
Contributor

Solved: it's apparently being caused by this stray unqualified type in a constructor parameter list (?!): godbolt.org/z/YnWb9nzKE

Removing the ctor declaration or qualifying the declaration with v_noabi resolves the GCC compilation error. I'm quite suspicious now that this may be a GCC compiler bug. 😵

Interestingly, I can repro similar errors on MSVC and Clang with some modifications:

Copy link
Contributor

@connorsmacd connorsmacd left a comment

Choose a reason for hiding this comment

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

LGTM

///
/// Equivalent to @ref type() const.
///
/// To support incremental migration to @ref bsoncxx::v1::types::view.
Copy link
Collaborator

@kevinAlbs kevinAlbs Sep 17, 2025

Choose a reason for hiding this comment

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

Suggested change
/// To support incremental migration to @ref bsoncxx::v1::types::view.
/// To support incremental migration to @ref bsoncxx::v1::types::value.

Are the added type, type_id and get_<type> methods intended for feature parity with bsoncxx::v1::types::value? I expect this may not strictly be needed to ease migration since existing applications would not have calls to these methods.

Copy link
Contributor Author

@eramongodb eramongodb Sep 17, 2025

Choose a reason for hiding this comment

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

Not for feature parity, but for name parity, so that when root namespace redeclarations are updated from v_noabi to v1, the required changes can be minimized in advance by downstream users without forced-breaking-changes:

bsoncxx::types::view v; // Syntax is the same for v_noabi and v1.
v.type_view();          // Syntax is the same for v_noabi and v1.

When bsoncxx::types::view is updated from v_noabi::types::view to v1::types::view, code which have already been incrementally updated (as part of a API minor version release) to use the new names will not be broken by the redeclaration release (as part of a API major version release) to the same extent as if the v_noabi new-name equivalents were not available.

@eramongodb eramongodb merged commit 91b9c64 into mongodb:master Sep 17, 2025
16 of 17 checks passed
@eramongodb eramongodb deleted the cxx-abi-v1-renames branch September 17, 2025 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants