Merged
Conversation
bd894d6 to
b4cfe79
Compare
hansenms
approved these changes
Dec 8, 2022
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.
Enabling a macOS smoke test job in the pipeline, similar to the Windows' one. To both smoke test scripts, we add the unit test model and make sure that the generated code compiles. This exposed an issue with compiling some of our generated code with MSVC. For HDF5, we delete the copy constructors for the "inner" types that are used to represent the data in a way that HDF5 supports. At the same time, we were implementing conversion operators to convert from the inner to outer types. Deep in the expansion of xtensor templates, we were getting errors that it was trying to use deleted copy constructors and assignment operators.
The solution is to move away from conversion operators and to instead pass a reference to an outer type to a
ToOuter()method and populate it.There were a few other minor changes that needed to be made in order to get the code compiling on Mac and Windows.