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.

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

[C++] Introduce synchronization and container abstractions #3694

Merged
merged 1 commit into from
May 5, 2022

Conversation

jcking
Copy link
Collaborator

@jcking jcking commented May 5, 2022

In most cases the synchronization and container implementations provided by the C++ standard library are sufficient. However, there are some environments where users may wish to substitute other implementations. One of those is Abseil. By default, the implementations provided by the C++ standard library are used and should be preferred.

@jcking
Copy link
Collaborator Author

jcking commented May 5, 2022

@hzeller

@jcking jcking force-pushed the cpp-easy-patching branch 3 times, most recently from 5e54a98 to 94be95a Compare May 5, 2022 15:52
@hzeller
Copy link
Contributor

hzeller commented May 5, 2022

Wasn't there also a std::call_once in ATNDeserializationOptions.cpp and in XPathLexer.cpp ?

@jcking
Copy link
Collaborator Author

jcking commented May 5, 2022

Wasn't there also a std::call_once in ATNDeserializationOptions.cpp and in XPathLexer.cpp ?

Whoops, thanks. Forgot about those.

@hzeller
Copy link
Contributor

hzeller commented May 5, 2022

LGTM
Can you double check and recursive grep for any of the replaced synchronization primitives (std::call_once, std::mutex, shared, unique lock etc) as mixing them might not be a good idea.

Some of the typical singleton initalizations with call_once might be replaced with the following pattern, which is safe since C++11 and arguably easier to read:

Foo* FooSingleton() {
   static Foo* instance = new Foo();
   return instance;
}

Signed-off-by: Justin King <jcking@google.com>
@GoToCoding
Copy link

Sorry for writing to finished PR.

Are you considering to fully move to abseil types instead of std?
Because this duplicating defines look little strange if you want to use abseil flatmap in more and more places.
[Of course that's just my opinion]

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