Skip to content

[CDRIVER-5916] Deprecate the Hedged-Reads APIs #1889

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

Merged
merged 6 commits into from
Feb 28, 2025

Conversation

vector-of-bool
Copy link
Contributor

@vector-of-bool vector-of-bool commented Feb 26, 2025

Background

Refer: CDRIVER-5916

DRIVERS-2929 is be a spec update that deprecates Hedge Reads in driver APIs. This changeset adds such deprecation warnings, among other things.

Summary

This does not change any library behavior, it only adds deprecation warnings, and updates those that already exist. Per-commit review is recommended:

  • c17b30f Adds new deprecation attribute macros that work on MSVC, to replace the BSON_GNUC_DEPRECATED macros. This macro always requires a deprecation message string, which will be included in the emitted attribute if it is supported by the compiler.
  • ef8347b Updates all existing deprecation attributes to use the new macros:
    • This also applies a formatting change and repositions the attributes for MSVC compatibility.
    • We also add several macros to .clang-format's AttributeMacros to format them as declaration specifiers on functions.
    • Lots of headers have had whitespace inserted between function declarations, where it was becoming nigh unreadable.
    • Because deprecation warnings are now applied on MSVC, a new deprecation-warning-suppression had to be added to bson-atomic.h and bson-cmp.h
    • This commit is the largest change, and was largely a find-replace job.
  • 0da23b0 Adds an internal declaration-like macro that emits the appropriate _Pragma to disable deprecation warnings.
  • 4cc8a97 Adds deprecation annotations to "hedge" APIs, and updates the documentation to note the deprecation. Warning suppressions were added in internal locations where the hedged-read APIs are called.

Note on Attribute Placement:

MSVC attributes are declaration-specifiers (thus "__declspec"), and thus need to be positioned correctly. The following is ill-formed:

int*
BSON_DEPRECATED("oof")
foo();

because the __declspec is placed between the asterisk and the function name (not a valid position for declaration specifiers). GCC accepts this form, but we cannot use it for compatibility. One possible correct formatting for this declaration would be:

int
BSON_DEPRECATED("oof")
* foo();

but that is a extremely weird, so it's easier to just place the deprecation before the return type:

BSON_DEPRECATED("oof")
int* foo();

This has been applied throughout the codebase in ef8347b.

We now emit deprecation warnings on MSVC as well, and all deprecation
warnings now include a string message, if possible.
- Replace use of `BSON_GNUC_DEPRECATED...` with `BSON_DEPRECATED...`
- Reformat attribute macros as attributes according to clang-format.
- Give dense API listings in public headers some breathing room.
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

The MSVC deprecation warnings and additional deprecation warning strings are much appreciated. LGTM with comments addressed.

@@ -532,7 +532,7 @@ bson_as_canonical_extended_json (const bson_t *bson, size_t *length);
* Returns: A newly allocated string that should be freed with bson_free().
*/
BSON_EXPORT (char *)
bson_as_json (const bson_t *bson, size_t *length) BSON_GNUC_DEPRECATED_FOR (bson_as_legacy_extended_json);
bson_as_json (const bson_t *bson, size_t *length);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add back deprecation warning:

BSON_DEPRECATED_FOR (bson_as_legacy_extended_json)
BSON_EXPORT (char *) bson_as_json (const bson_t *bson, size_t *length);

@vector-of-bool vector-of-bool changed the title Deprecate the Hedged-Reads APIs [CDRIVER-5916] Deprecate the Hedged-Reads APIs Feb 28, 2025
@vector-of-bool vector-of-bool marked this pull request as ready for review February 28, 2025 18:49
@vector-of-bool vector-of-bool merged commit d6f975d into mongodb:master Feb 28, 2025
42 of 43 checks passed
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.

2 participants