build: MSVC workaround for empty base classes#730
Merged
mpusz merged 1 commit intompusz:masterfrom Oct 9, 2025
Merged
Conversation
mpusz
approved these changes
Oct 9, 2025
Owner
mpusz
left a comment
There was a problem hiding this comment.
Thank you! I wasn't aware of it and had to fix many similar lines in mp-units before to make MSVC happier. I was changing u to U{} and similar everywhere. Units are not the only ones affected. Similar issues may happen with dimension and quantity_spec classes, as they have the same design and usage. If you didn't approach problems so far, it means that I've probably fixed most of those already 😉
If only I had known about this workaround, it would have been much easier to get the MSVC compliance earlier.
Owner
|
BTW, I really appreciate the MSVC developers' efforts here! 💜 |
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
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.
MSVC is knocking out bugs to get
mp-unitsbuilding. One of those bugs occurs here inunit.h:The call to
get_canonical_unittries to copyu, which is outside the evaluation context ofcanonical_u. Normally this would be an error, but sinceUnittypes have no members to copy (they are "empty"), this is actually permitted by the C++ standard.We get this wrong due to an age-old layout problem. The structs
scaled_unit_implandderived_unit_implinherit from empty base classes, so they should be considered empty as well. But due to historical reasons, we don't grant this if there are multiple base classes.This is fixable on the compiler side. In the meantime, adding
__declspec(empty_bases)is the workaround to get standard behavior (MSVC docs, introduced in Visual Studio 2015).