-
Notifications
You must be signed in to change notification settings - Fork 277
[Hdf5] Fix windows compilation #14091
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
I'm compiling this branch, will let you know |
|
I am getting an Here's the log: |
|
Ok, compilation errors should be solved, but not sure about the CI testes failing, I suspect there is some mismatch with the overload of the cheks in the function |
|
I am still getting some Here's the log: |
sunethwarna
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @roigcarlo , and sorry for being late....
| namespace Internals | ||
| { | ||
| std::string AddMissingAndGetPrefix(Parameters Settings) | ||
| std::string AddMissingAndGetPrefix(Parameters & Settings) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, otherwise the copy contructor implicitly calls Get<>
| KRATOS_TRY | ||
|
|
||
| const std::string prefix = Settings["prefix"].Get<std::string>(); | ||
| const std::string prefix = Settings["prefix"].GetString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious: Was the previous causing problems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getstd::string is not exposed, hence cannot be called.
📝 Description
@Rbravo555 this Fixes #14089 (or should, can't test exaustively). If you can try it and give feedback I'd appreaciate.
@sunethwarna There is big change in hdf5_points_data. I've merged the source and header in one file. I am afraid that there is no other solution for this class in particular, because
VertexContainerCoordinateIOis derived fromPointsData<VertexIO>which causes the later to be partialy defined until the compiler sees the definition ofPointsData<VertexIO>, which triggers the error and the explicit instantiation cannot be moved before the definition ofVertexContainerbecause its base class needs to come before...Rest of the problems were most warnings because stf::vector especializations which causes functions not being able to resolve overloads correctly.
That being said, I reported this to be broken about a year ago.