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.

Sign up for GitHub

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

feat(common): adds begin/end inline namespace macros #7456

Merged
merged 2 commits into from
Oct 13, 2021
Merged

feat(common): adds begin/end inline namespace macros #7456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
739f362
feat(inline-ns): adds begin/end inline namespace macros
devjgm Oct 13, 2021
6708a24
add Doxygen macro overrides to keep it working during interim steps
devjgm Oct 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmake/GoogleCloudCppCommon.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ elseif (GOOGLE_CLOUD_CPP_GENERATE_DOXYGEN)
"GOOGLE_CLOUD_CPP_IAM_DEPRECATED(x)="
"GOOGLE_CLOUD_CPP_BIGTABLE_IAM_DEPRECATED(x)="
"GOOGLE_CLOUD_CPP_SPANNER_ADMIN_API_DEPRECATED(x)="
"GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN=inline namespace omit_this_inline_ns {"
"GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END=}"
Comment on lines +83 to +84
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like these would work as (and be better as) EXPAND_AS_DEFINED names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to keep them as they are because this is only temporary and the actual change I want to make in about 2 PRs is to change them to be defined as empty string, which should be done like this.

"GOOGLE_CLOUD_CPP_NS=omit_this_inline_ns")
set(DOXYGEN_HTML_TIMESTAMP YES)
set(DOXYGEN_STRIP_FROM_INC_PATH "${PROJECT_SOURCE_DIR}")
Expand Down
18 changes: 18 additions & 0 deletions google/cloud/version.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,24 @@
GOOGLE_CLOUD_CPP_VEVAL(GOOGLE_CLOUD_CPP_VERSION_MAJOR, \
GOOGLE_CLOUD_CPP_VERSION_MINOR)

/**
* Versioned inline namespace that users should generally avoid spelling.
*
* The actual inline namespace name will change with each release, and if you
* use it your code will be tightly coupled to a specific release. Omitting the
* inline namespace name will make upgrading to newer releases easier.
*
* However, applications may need to link multiple versions of the Google Cloud
* C++ Libraries, for example, if they link a library that uses an older
* version of the libraries than they do. This namespace is inlined, so
* applications can use `google::cloud::Foo` in their source, but the symbols
* are versioned, i.e., the symbol becomes `google::cloud::vXYZ::Foo`.
*/
#define GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN \
inline namespace GOOGLE_CLOUD_CPP_NS {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we've limited use of GOOGLE_CLOUD_CPP_NS, perhaps it is time to slip INLINE (or something) in there. At the moment it sounds like it refers to google::cloud.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could do that separately. To avoid breaking users, we'd need to keep the old macro name around for a while at least.

#define GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END \
} // namespace GOOGLE_CLOUD_CPP_NS

// This preprocessor symbol is deprecated and should never be used anywhere. It
// exists solely for backward compatibility to avoid breaking anyone who may
// have been using it.
Expand Down