Make structs and unions adhere to C++ standard #101
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
gostefan
force-pushed
the
fixUnion
branch
2 times, most recently
from
1c2291b
to
6e31208
Compare
|
Force-update because of the fix to PR #99. The last two commits have the same contents. |
|
Fixed another issue in PR #99 and needed to force-push. The last two commits still contain the same changes. |
The setup before this commit violates the C++ standard. If two classes with non-standard constructors and/or destructors are in a union their constructors and/or destructors have to be called explicitly when switching between the value types. Using a union is also unsafe as it's illegal to access the "wrong" of the two members. But the API doesn't have a way to tell which member is the active. The new solution doesn't use unions but allows clients to safely access even the wrong value.
Accessing non-active members in unions is undefined behavior. Additionally the C++ Standard doesn't support anonymous structs. [1] This puts the current implementation of TVec3 and TVec4 square into undefined behavior territory. Removing the unions and anonymous structs is easy and fixes all problematic paths. [1]: https://stackoverflow.com/questions/2253878/why-does-c-disallow-anonymous-structs
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.
Some structs and unions currently don't adhere to the C++ standard.
They rely solely on the compiler implementation doing what we expect it to do.
This PR fixes the elements that don't adhere to the C++ standard.
Only the last to commits are actually part of this PR. All the others are from PR #99 but not basing this PR on that one would lead to conflicts when merging.