Various cleanups: Use unnamed namespaces #4124
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `constexpr` variables and `static` functions had internal linkage, so this is unobservable to other TUs. We can drop `static` now.
UCRT `<new.h>` declares `_set_new_handler`, which is better than trying to declare it ourselves. We can then drop the `new_hand` typedef which was being used for that declaration. The `_New_handler` variable was `static`, so we can clearly move it into an unnamed namespace and drop the `static`. We do need to qualify the `_STD new_handler` type now. The `_New_handler_interface` helper function was never dllexported nor referred to by other stl/src TUs. We can move it out of `namespace std` and into the unnamed namespace. The resulting code has our desired distinction between TU-local and dllexported machinery.
CaseyCarter
approved these changes
Oct 24, 2023
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
|
I've resolved the simple merge conflict in |
CaseyCarter
reviewed
Oct 26, 2023
| @@ -46,7 +46,8 @@ namespace { | |||
|
|
|||
| _EXTERN_C | |||
|
|
|||
| _CRTIMP2_PURE long long __cdecl _Xtime_get_ticks() { // get system time in 100-nanosecond intervals since the epoch | |||
| _CRTIMP2_PURE long long __cdecl _Xtime_get_ticks() noexcept { | |||
| // get system time in 100-nanosecond intervals since the epoch | |||
There was a problem hiding this comment.
It's been hard work causing all the merge conflicts on my own since @BillyONeal left the team.
CaseyCarter
approved these changes
Oct 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(I recommend "ignore whitespace" while reviewing this.)
Over time, we've developed a convention of using unnamed namespaces in
stl/srcfor internal machinery. This clearly separates TU-local code from functions that are accessed from otherstl/srcTUs, injected into the import lib, or dllexported. I noticed a couple of older TUs that could benefit from being cleaned up.xtime.cpp: The_Epochand_Nsec100_per_secconstants can be function-local.xtime.cpp: Use an unnamed namespace, following our modern convention.constexprvariables andstaticfunctions had internal linkage, so this is unobservable to other TUs. We can dropstaticnow.stdhndlr.cpp: Use an unnamed namespace, following our modern convention.<new.h>declares_set_new_handler, which is better than trying to declare it ourselves. We can then drop thenew_handtypedef which was being used for that declaration._New_handlervariable wasstatic, so we can clearly move it into an unnamed namespace and drop thestatic. We do need to qualify the_STD new_handlertype now._New_handler_interfacehelper function was never dllexported nor referred to by otherstl/srcTUs. We can move it out ofnamespace stdand into the unnamed namespace.