Skip to content
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

with c++11 mode ON, clang complains about how we use typeid in itk-mhd #58

Closed
djboersma opened this issue Feb 8, 2016 · 1 comment
Closed

Comments

@djboersma
Copy link
Contributor

This is an issue in the "nitpick" category.

Building GATE on MacOSX El Capitan with nomt G4.10.02, using clang; switching C++11 mode ON in cmake, the build is almost clean, except for this warning:

/Users/montecarlo/Software/CERN/GATE/Gate/source/externals/itk-mhd/MetaIO/metaOutput.cxx:579:17: warning: expression with side effects will be evaluated despite being used as an operand to 'typeid' [-Wpotentially-evaluated-expression]
      if(typeid(*(*itStream)) == typeid(MetaFileOutputStream))
                ^

I see that typeid is used to do something that could be done way more elegantly in a proper OO manner. A small, still hackish fix would be to dynamically cast *itStream to a MetaFileOutputStream pointer, and depending on whether that cast succeeds we call this->GenerateXML(...) with or without filename argument. By using that dynamically casted pointer we don't need that C-style static cast anymore either, in line 581.

However, this code was not written by Gate developers, it was copied from ITK or VTK. I see that it is perfectly identical to MetaIO/metaOutput.cxx in ITK 4.7.1 and VTK 5.10.1. However, Kitware seems to have run into the same warnings, because in ITK 4.8.1 (and VTK 6.3.0) they have indeed replaced the typeid by a dynamic cast (but they still do the ugly C-style cast).

So I guess we should not try to patch this ourselves, but rather update the Gate MetaIO directory by copying the MetaIO directory of a more recent release of VTK. How (un)comfortable would you all be with such an operation? A recursive diff of MetaIO in current Gate/develop and VTK 6.3.0 is not terribly long, and to me most changes seem to me of the code-gardening variety.

(Of course this whole point is moot if the user has already ITK/VTK installed on her machine and can use that instead of the "external", but that is a separate discussion.)

@dsarrut
Copy link
Contributor

dsarrut commented Feb 9, 2016

Right David, MetaIo is copied from ITK and updated from time to time. Currently, I have no time to do this copy and test, but we should plan it for the next release.

@dsarrut dsarrut closed this as completed Jun 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants