Add ODR detection to WINRT_NO_MAKE_DETECTION #735
Merged
Conversation
This file contains hidden or 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
Mismatching `WINRT_NO_MAKE_DETECTION` results in inconsistent vtables and very confusing runtime crashes. Detect the problem at link time. `pragma detect_mismatch` must occur at global scope, so we float it to the top of `base_implements.h` even though it would more logically be placed at the point of declaration of the bonus virtual member. This change uncovered the fact that `test` and `test_win7` had portions built with and without `WINRT_NO_MAKE_DETECTION`, so it was already violating ODR violation. Moved the `WINRT_NO_MAKE_DETECTION` to the test component and make the entire test component enable `WINRT_NO_MAKE_DETECTION`. The symbol `WINRT_LEAN_AND_MEAN` does not require ODR protection because it excludes entire functions and classes, rather than rewriting pieces of existing functions and classes. Technically, `WINRT_IMPL_IUNKNOWN_DEFINED` is also subject to ODR due to how it changes the definition of some type traits classes some types in the `winrt::impl` namespace, and activates some member functions and specializations. In practice, this isn't a problem because the type traits classes are constexpr (and therefore follow the settings of the current translation unit), and the other issues, if conflicting, trigger linkage errors.
kennykerr
approved these changes
Aug 26, 2020
ChrisGuzak
approved these changes
Sep 1, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Mismatching
WINRT_NO_MAKE_DETECTIONresults in inconsistent vtables and very confusing runtime crashes. Detect the problem at link time.pragma detect_mismatchmust occur at global scope, so we float it to the top ofbase_implements.heven though it would more logically be placed at the point of declaration of the bonus virtual member.This change uncovered the fact that
testandtest_win7had portions built with and withoutWINRT_NO_MAKE_DETECTION, so it was already violating ODR. Moved theWINRT_NO_MAKE_DETECTIONto the test component and make the entire test component enableWINRT_NO_MAKE_DETECTION.The symbol
WINRT_LEAN_AND_MEANdoes not require ODR protection because it excludes entire functions and classes, rather than rewriting pieces of existing functions and classes.Technically,
WINRT_IMPL_IUNKNOWN_DEFINEDand__IInspectable_INTERFACE_DEFINED__are also subject to ODR due to how it changes the definition of some type traits classes some types in thewinrt::implnamespace, and activates some member functions and specializations. In practice, this isn't a problem because the type traits classes are constexpr (and therefore follow the settings of the current translation unit), and the other issues, if conflicting, trigger linkage errors.