CXX-2796 Restore external polyfill library and stdlib support

[eramongodb]

Description

This PR partially resolves CXX-2796. Full resolution pending #1075. (Merged.) Verified by this patch.

This PR restores polyfill library (pre-C++17) and stdlib (post-C++17) support, including the explicit setting of a BSONCXX_POLY_USE_* option regardless of the selected C++ standard. This avoids the pending r3.10.0 release from potentially causing source-breaking changes in downstream code that depends on expectations of the underlying type for bsoncxx::stdx::string_view or bsoncxx::stdx::optional<T> being consistent with a selected polyfill library.

BSONCXX_USE_POLY_IMPLS

This PR adds a new CMake option BSONCXX_POLY_USE_IMPLS to opt-into using bsoncxx implementations for polyfills. This configuration is denoted as i in the ABI tag. The ENABLE_BSONCXX_POLY_USE_IMPLS option controls whether bsoncxx implementations are considered by default polyfill library selection (when no polyfill library is explicitly selected by the user).

ENABLE_BSONCXX_POLY_USE_IMPLS is currently OFF by default to avoid source-breaking changes in user code that depends on current default polyfill library selection behavior for pre-C++17 configurations. This option is expected to be set to ON by default in an upcoming major release as part of CXX-2797. Installation documentation is updated to recommend users set BSONCXX_POLY_USE_IMPLS=ON to avoid external library dependencies. The external polyfill library options are now deprecated in favor of bsoncxx implementations per CXX-2797. CMake deprecation warning messages are also added accordingly.

Note, BSONCXX_POLY_USE_IMPLS=ON is currently broken due to #1075 not yet being merged (there is no bsoncxx implementation of std::optional).

Restoring Polyfill Support

This PR restores polyfill selection behavior and support to be consistent with the r3.9.0 release.

This was fairly straightforward for string_view.hpp, which, relative to r3.9.0 (sans using-decl refactor changes), simply appends the BSONCXX_POLY_USE_IMPLS case to the list of alternatives before the #error case:

  /* ... */ // (Mostly) unchanged relative to 3.9.0.

  #elif defined(BSONCXX_POLY_USE_STD)
  
  #include <string_view>
  
  namespace bsoncxx {
  namespace v_noabi {
  namespace stdx {
  
  using ::std::basic_string_view;
  using ::std::string_view;
  
  }  // namespace stdx
  }  // namespace v_noabi
  }  // namespace bsoncxx

+ #elif defined(BSONCXX_POLY_USE_IMPLS)
+ 
+ /* ... */ // bsoncxx implementation of std::string_view.
+ 
  #else
  #error "Cannot find a valid polyfill for string_view"
  #endif

This pattern is expected to be applied similarly to the optional.hpp header after #1075 is merged in a followup PR.

The make_unique.hpp header is different. Because it only provides non-exported function templates, its impact on source-compatibility is minimal (situations where our changes can break source would be considered abnormal usage of these interfaces). Furthermore, r3.9.0 introduced a C++20 polyfill for make_unique_for_overwrite<T>() which is not covered by the usual C++17 external polyfill libraries. Because neither of these features are related to C++17-compatibility, this PR proposes using feature test macros for both make_unique<T>() (C++14) and make_unique_for_overwrite<T>() (C++20) to select bsoncxx vs. stdlib implementations, independent of the C++17 polyfill library configuration.

Restoration of r3.9.0 behavior can be verified by the ABI stability reports for both polyfill and stdlib configurations (compared to those in the latest commit for polyfill and stdlib).

Miscellaneous

• Disable string_view tests for non-bsoncxx polyfills due to compatibility and conformance issues. We only need the tests to test our implementations anyways (+ stdlib to verify test consistency).
• Update abi-compliance-check-test.sh to ignore detail namespaces in compatibility reports to reduce noise.
• Fix the abidiff-test.sh script so it actually uses common flags as intended. Removed the --non-reachable-types flag due to producing undesirable errors beyond what we want to test.

[kevinAlbs]

LGTM with minor changes.

[eramongodb]

PR has been updated following merge of #1075. This PR now fully resolves CXX-2796.

Latest changes verified by this patch.

Notable changes:

• Restore polyfill selection for bsoncxx::stdx::optional

Use of mnmlstc/core and Boost equivalents as polyfills is restored to r3.9.0 behavior (sans experimental stdlib, removed by 8e1c47a).

For better consistency with other polyfills, the Boost polyfill was updated to conditionally include a std::in_place_t equivalent (this is new), as the minimum Boost version of 1.56.0 doesn't provide it. boost::in_place_init_t is a 1.63.0+ feature. Alternatively, we could bump the minimum Boost version to 1.63.0 and remove the condition.

• Only enable optional<T> tests for bsoncxx and stdlib polyfills

Same as what was done for string_view. We're only interested in testing the quality of the bsoncxx implementation against the stdlib implementations.

• Relaxed Type Trait Assertions for optional<T>

In addition to improving the error messages for type trait equality assertions on failure, when testing against stdlib implementations, the equality assertions are relaxed to assert implication instead. This is to account for deviations in stdlib behavior in the negative cases (e.g. https://godbolt.org/z/P5ab6W46W). Instead of Trait<T> == Trait<optional<T>>, we instead test Trait<T> -> Trait<optional<T>> (implication). Equality is still used when testing the bsoncxx implementation.

• scoped_bson_t ctors

Rather than conditional inclusion of potentially-ambiguous overloads based on polyfill configuration, explicit overloads for all view/value/view_or_value arguments are defined instead to address the potential ambiguity issues.

• Drive-by: improve abidiff test script

Some changes were made to reduce inclusion of unrelated symbols in the diff report.

• Drive-by: CCACHE_BASEDIR on Windows

Overlooked that config-time CMake paths must be Windows-based.

• Drive-by: BUILD_TESTING as CMake option

The BUILD_TESTING normal variable is now a cache variable that can be toggled via CMake GUI, ccmake, etc.

[kevinAlbs]

LGTM. Nicely done. The test improvements are appreciated.

[kevinAlbs]

Suggested change

	// Constructs a new scoped_bson_t from a document view_or_value.
	// Constructs a new scoped_bson_t from a document.

[eramongodb]

Another drive-by fix: added missing macro guards for BSONCXX_POLY_USE_STD.

[vector-of-bool]

I'm not sure what the point of all of my work was if we're just bringing back all the old polyfill support anyway. Why did I do anything at all if it just adds another configuration that we need to test? @kevinAlbs @eramongodb

[kevinAlbs]

if we're just bringing back all the old polyfill support anyway.

@vector-of-bool I expect restoring the polyfills prevents possible source breaking changes. Quoting from a slack conversation with @eramongodb (which I should have included in the ticket):

// User expectation: BSONCXX_POLY_USE_BOOST=ON -> bsoncxx::stdx::string_view is boost::string_view.
// Same assumption applies to other polyfill library configurations.
void user::impl(boost::string_view sv) { /* ... */ }

void user::code() {
  auto sv = bsoncxx::stdx::string_view("abc");
  user::impl(sv); // error: breaking change in 3.10 (without CXX-2796 or with CXX-2697).
}

We do not have much documentation concerning our expectations of how users use our polyfills except a line about std::experimental being discouraged and unsupported (despite us having EVG tasks for it), and this line (despite us not supporting ABI stability

DO NOT change your project's polyfill if you need to create a stable binary interface.

So the user expectation above is justified even if not desirable by us.

The removal of the other polyfills (CXX-2797) is marked for the next major release (4.0.0). That said, I think think the bsoncxx polyfills provide value in the 3.x releases. Users on pre C++17 compilers can use the bsoncxx polyfills to avoid using Boost / MNMLSTC.

[eramongodb]

@vector-of-bool This PR does not invalidate the work done on bsoncxx polyfill implementations. We do want to exclusively use the bsoncxx implementation for pre-C++17 configurations and drop external polyfill libraries. We just can't do it right now, for the upcoming 3.10 release, because that would violate our commitment to SemVer API versioning by introducing source-breaking changes in a minor version release. Recent interest and efforts to schedule a 4.0 major version release is primarily and directly motivated by our desire to resolve CXX-2797 "Remove MNMLSTC and Boost from CMake scripts and docs". Most of the changes in this PR are expected to be reverted or obsoleted by the 4.0 release as part of CXX-2797.

To that end, this PR makes efforts to drive migration by users to using the bsoncxx implementations in the 3.10 release by adding CMake deprecation messages to all external library polyfill configurations, either via ENABLE_BSONCXX_POLY_USE_IMPLS=ON (pre-C++17 default) or BSONCXX_POLY_USE_IMPLS=ON (unconditional, even post-C++17). It also explicitly deprecates all external polyfill library configurations in documentation. This is all consistent with the Drivers Maintenance & Compatibility Policy for handling API breaking changes:

Backward breaking API changes will only occur when incrementing the semver major version (X). An API will be documented as deprecated for at least one semver minor version (Y) before a breaking change occurs. When possible, a deprecation warning will be raised when an application uses any deprecated API. [...] It should be possible to upgrade to the last minor release before the major release, address all the deprecation warnings, and then upgrade to the major release without any further application changes.

This PR (1) documents external polyfill library configurations as deprecated for the upcoming 3.10 minor version, (2) raises CMake deprecation warnings for such configurations, and (3) provides opt-in configuration settings to address deprecation warnings and upgrade to using the bsoncxx polyfill implementation before the upcoming 4.0 major version release when the breaking change occurs.

It is in our best interest to schedule the 4.0 release as soon as able so that we can finally address polyfill library configuration complexity and resolve CXX-2797; this PR ensures we are not violating SemVer API versioning requirements along the way.

[vector-of-bool]

Sorry for the delay, and apologies for the brief existential dread. LGTM, with only a few minor comments/questions.

[vector-of-bool]

Is there an explanation on why these constructors needed to be revived? I'm guessing that one or more of the optional impls doesn't "do the right thing"?

[eramongodb]

Correct. Rather than dealing with inconsistent optional<T> converting constructor behavior across configurations, settled on making it definitively unambiguous on all implementations for all view/value/view_or_value/optional<view_or_value> arguments expected to be given.

[vector-of-bool]

Could this section -- repeated between {linux,macos,windows}.md -- be refactored into a separate page?

[eramongodb]

Good idea. Added a new mongocxx-v3/polyfill-selection page that is linked to from the per-OS installation pages.

[vector-of-bool]

I'm iffy on this CMake setting name. It is unclear what it means compared to USE_BOOST, USE_STD, USE_MNMLSTC etc. I'm unfortunately not sure on what a better name might be.

[eramongodb]

I went back and forth several times on what to name this configuration and settled on "bsoncxx implementations" -> "IMPLS" due to the ABI tag parameter b already being used for Boost. This way i can be used for "(bsoncxx) implementations". The hope is that descriptions and documentation will be enough to make its purpose/meaning clear.

[eramongodb]

I must admit I do not understand how the value of some of these fields are supposed to be set (date and weight in particular), so simply copied the template from similar files.