Consistent Export Symbol Proceedures: Request for comment.

Engineering
1

= Background

Various early attempts to fix problems with dynamic_casting seems to have resulted in various approaches to how exporting symbols is defined in macros.

= My understanding

We should constently use the partially implemented https://cmake.org/cmake/help/v3.10/module/GenerateExportHeader.html

Use the target properties CXX_VISIBILITY_PRESET and VISIBILITY_INLINES_HIDDEN can be used to add the appropriate compile flags for targets. See the documentation of those target properties, and the convenience variables CMAKE_CXX_VISIBILITY_PRESET and CMAKE_VISIBILITY_INLINES_HIDDEN.

** In its current state, there is a mix of cmake behavior and various customized one-off solutions that can be inconsistent with each other and cmake.

== OPTION 1 (This is what I think we should do)
Use GenerateExportHeader to generate a single ITKExport.h file at config time, include that in itkMacro.h and remove both

Generate a single ITK_TEMPLATE_EXPORT

currently we include files like (#include “ITKIOImageBaseExport.h”) but we don’t use any of those defined macros, we use ITK_TEMPLATE_EXPORT instead.

== OPTION 2 . (this is where I started with this PR but stopped for discussion first.
Generate one export header per library and include the files as necessary.

current generate header : number of includes
ClientTestLibraryAExport.h : 1
ClientTestLibraryBExport.h : 1
ClientTestLibraryCExport.h : 1
ITKBiasCorrectionExport.h : 2
ITKCommonExport.h : 1
ITKIOBMPExport.h : 2
ITKIOBioRadExport.h : 2
ITKIOBrukerExport.h : 2
ITKIOCSVExport.h : 1
ITKIOGDCMExport.h : 3
ITKIOGEExport.h : 9
ITKIOGIPLExport.h : 2
ITKIOHDF5Export.h : 2
ITKIOIPLExport.h : 3
ITKIOImageBaseExport.h : 15
ITKIOJPEG2000Export.h : 2
ITKIOJPEGExport.h : 2
ITKIOLSMExport.h : 2
ITKIOMINCExport.h : 2
ITKIOMRCExport.h : 3
ITKIOMeshBYUExport.h : 3
ITKIOMeshBaseExport.h : 5
ITKIOMeshExport.h : 16
ITKIOMeshVTKExport.h : 3
ITKIOMetaExport.h : 4
ITKIONIFTIExport.h : 2
ITKIONRRDExport.h : 2
ITKIOPNGExport.h : 2
ITKIOSiemensExport.h : 2
ITKIOStimulateExport.h : 2
ITKIOTIFFExport.h : 3
ITKIOTransformBaseExport.h : 5
ITKIOTransformHDF5Export.h : 2
ITKIOTransformInsightLegacyExport.h : 2
ITKIOTransformMatlabExport.h : 2
ITKIOVTKExport.h : 2
ITKIOXMLExport.h : 6
ITKKLMRegionGrowingExport.h : 4
ITKLabelMapExport.h : 1
ITKMeshExport.h : 2
ITKOptimizersExport.h : 31
ITKOptimizersv4Export.h : 7
ITKPathExport.h : 0
ITKPolynomialsExport.h : 1
ITKQuadEdgeMeshExport.h : 1
ITKSpatialObjectsExport.h : 54
ITKStatisticsExport.h : 11
ITKTransformExport.h : 41
ITKVTKExport.h : 1
ITKVideoCoreExport.h : 3
ITKVideoIOExport.h : 4
ITKWatershedsExport.h : 2

= Other code places where symbol exporting is done differently:

1 Like
2

Hans,

This is a very complicated issue. I’m not sure you are heading down the proper path. There are several use cases here and many different behaviors of compilers and linkers that needs to be considered.

Do you have a specific situation where this problem has come up again?

The patch you revived was really old, before the current solution was implemented.

Sorry for the brief answer today.

3

As @blowekamp notes, this is a complex issue, and there are multiple states to account for regarding ITK’s build configuration, the client application build configuration, and the target operating system. More details on the content and history are described below.

This is not correct. There is not a mix of CMake behavoir and customized one-off solutions. The CMake behavior is extended beyond what is available in CMake’s GenerateExportHeader.cmake and the VISIBILITY variables.

ITK’s ITK_TEMPLATE_EXPORT and ITK_FORWARD_EXPORT functionality is not available in GenerateExportHeader. In the initial implementation, we considered extending GenerateExportHeader and started down that path, but after considering that,

  1. These two macros do not depend on BUILD_SHARED_LIBS status, and they do not require modification at configuration time like some of the other export macros.
  2. We can avoid the need for the generation of the export header in most modules that are header only.
  3. Forking GenerateExportHeader properly would take some work to namespace it to ensure it is used when required and not used when not intended.

they were placed in itkMacro.h.

There is a single ITK_TEMPLATE_EXPORT now.

ITKIOImageBaseExport.h defines the exports for non-templated classes.

To make their purpose more explicit, the definitions of ITK_TEMPLATE_EXPORT, and ITK_FORWARD_EXPORT could be moved to a dedicated file, ITK/Modules/Core/Common/include/itkExport.h. @hjmjohnson do you think this would be helpful?

This should be avoided because it would require more include directories added to application builds. For large applications, e.g. 3D Slicer, this causes issues on Windows because of command line length limitations. Most ITK modules are header-only, and they only require one include directory from the source tree.

[quote=“hjmjohnson, post:1, topic:1453”]

github.com

InsightSoftwareConsortium/ITK/blob/225da5a4a0f1ae973bf349f1986c611b60a2c2ab/Modules/Core/Transform/include/itkTransformBase.h#L141-L173

/** Explicit instantiations */
#ifndef ITK_TEMPLATE_EXPLICIT_TransformBase
// Explicit instantiation is required to ensure correct dynamic_cast
// behavior across shared libraries.
//
// IMPORTANT: Since within the same compilation unit,
//            ITK_TEMPLATE_EXPLICIT_<classname> defined and undefined states
//            need to be considered. This code *MUST* be *OUTSIDE* the header
//            guards.
//
#  if defined( ITKTransform_EXPORTS )
//   We are building this library
#    define ITKTransform_EXPORT_EXPLICIT ITK_TEMPLATE_EXPORT
#  else
//   We are using this library
#    define ITKTransform_EXPORT_EXPLICIT ITKTransform_EXPORT
#  endif
namespace itk
{
#ifdef ITK_HAS_GCC_PRAGMA_DIAG_PUSHPOP

Export specifications are different depending on whether it is a non-templated class, templated class implicity instantiated, forward declaration, or templated class explicitly instantiated.

1 Like
4

Thanks. The behavior is very very complicated. When I did the tracings of what the macro to macro to macro to visibility setting, I thought I had identified something less convoluted.

I tried to understand the content on referred to: https://itk.org/Wiki/ITK/FAQ#What_is_the_.22ITK_TEMPLATE_VISIBILITY_DEFAULT.22_CMake_option.3F but that information was not very helpful :slight_smile:

In looking at the git blame logs, it appears that many of the codeblocks that have common programming style that appear to were done by different people at different times. I was investigating if a single approach could perhaps simplify the exporting mechanisms.

I did something similar in VXL by using generate_export_header, and it GREATLY reduced the complexity there. Perhaps there a bugs in VXL as well.

To be honest, I read a lot of code today, and I still don’t understand why it needs to be so complicated.

Things like “This code MUST be OUTSIDE the header guards” but no explanation why. I can not figure out why that is necessary. Then there are conditionals that define new macros conditional to values of other macros in ways that appear to mimic the behavior of the generate_export_header and when tracing those back it is not clear why two values are needed.

I admit that I don’t fully understand what is going on, but after work all day on it, I’ve not found documentation or a rational for what is happening.

I’m going to abandon this work for now and close all the patches.

5

It is, indeed, very complicated. There are many different configurations that require handling in a different way. And it is difficult to reproduce the different use cases when testing locally. Ultimately, the need for the different macros comes down to the fact that different operating systems have binary formats and binary loaders that handle the different types of symbols in different ways. There are a lot of different’s; the reality is that we need to accommodate them to work with the use cases.

More context and background can be found in the Gerrit review discussions, but there certainly could be better consolidated documentation that explain the rationale behind the current macros.

@fbudin gave a great presentation on related binary issues and debugging at our local C++ MeetUp; he used some of these skiils to address some the different’s we have tackled. Also related and possibly of interest is Matt Goldbolt’s 2018 CppCon talk on Linux binaries.