Skip to content

Commit

Permalink
Fix documentation glitches and symbol visibility table (#294)
Browse files Browse the repository at this point in the history 
Signed-off-by: Cary Phillips <[email protected]>
  • Loading branch information
@cary-ilm
cary-ilm committed Feb 28, 2023
1 parent 3112db4 commit 7b8547c
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 55 deletions.
130 changes: 79 additions & 51 deletions docs/SymbolVisibility.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
SPDX-License-Identifier: BSD-3-Clause
Copyright Contributors to the OpenEXR Project.
.. _Symbol Visibility in OpenEXR:
.. _Symbol Visibility in Imath/OpenEXR:

Symbol Visibility in Imath/OpenEXR
##################################
Expand Down Expand Up @@ -63,61 +63,89 @@ it also exports the types for the member types as well, which is not
desired, so these are marked as N/A even though the compiler does
allow that to happen.


+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
| C++ vs Compiler | MSVC | mingw | gcc | clang |
+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
| function | ``dllexport/dllimport`` | ``dllexport/dllimport`` | ``visibility("default")`` | ``visibility("default")`` |
+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
| hide a function | N/A | N/A | ``visibility("hidden")`` | ``visibility("hidden")`` |
+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
| ``class(typeinfo)`` | N/A | N/A | ``visibility("default")`` | ``visibility("default")`` |
+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
| template class | N/A | N/A | ``visibility("default")`` | ``type_visibility("default")`` |
+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
| template data | N/A | N/A | ``visibility("default")`` | ``visibility("default")`` |
+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
| class template<br> instantiation | ``dllexport/dllimport`` | N/A | N/A | ``visibility("default")`` |
+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
| enum | N/A | N/A | auto unhides (N/A) | ``type_visibility("default")`` |
+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
| extern template | N/A | ``dllexport/dllimport`` | ``visibility("default")`` | ``visibility("default")`` |
+----------------------------------+-------------------------+-------------------------+---------------------------+--------------------------------+
.. list-table::
:header-rows: 1
:align: left

* - C++ vs Compiler
- MSVC
- mingw
- gcc
- clang
* - function
- ``dllexport/dllimport``
- ``dllexport/dllimport``
- ``visibility("default")``
- ``visibility("default")``
* - hide a function
- N/A
- N/A
- ``visibility("hidden")``
- ``visibility("hidden")``
* - ``class(typeinfo)``
- N/A
- N/A
- ``visibility("default")``
- ``visibility("default")``
* - template class
- N/A
- N/A
- ``visibility("default")``
- ``type_visibility("default")``
* - template data
- N/A
- N/A
- ``visibility("default")``
- ``visibility("default")``
* - class template instantiation
- ``dllexport/dllimport``
- N/A
- N/A
- ``visibility("default")``
* - enum
- N/A
- N/A
- auto unhides (N/A)
- ``type_visibility("default")``
* - extern template
- N/A
- ``dllexport/dllimport``
- ``visibility("default")``
- ``visibility("default")``

With this matrix in mind, we can see the maximal set of macros we need to
provide throughout the code. *NB*: This does not mean that we need to
declare all of these, just that they might be needed. `XXX` should be
substituted for the particular library name being compiled.

+----------------------------------+------------------------------------------------------------------+
| macro name | purpose |
+----------------------------------+------------------------------------------------------------------+
+----------------------------------+------------------------------------------------------------------+
| ``XXX_EXPORT`` | one of export or import for windows, visibility for others |
+----------------------------------+------------------------------------------------------------------+
| ``XXX_EXPORT_TYPE`` | for declaring a class / struct as public (for typeinfo / vtable) |
+----------------------------------+------------------------------------------------------------------+
| ``XXX_HIDDEN`` | used to explicitly hide, especially members of types |
+----------------------------------+------------------------------------------------------------------+
| ``XXX_EXPORT_TEMPLATE_TYPE`` | stating the template type should be visible |
+----------------------------------+------------------------------------------------------------------+
| ``XXX_EXPORT_EXTERN_TEMPLATE`` | exporting template types (i.e. extern side of extern template) |
+----------------------------------+------------------------------------------------------------------+
| ``XXX_EXPORT_TEMPLATE_INSTANCE`` | exporting specific template instantiations (in cpp code) |
+----------------------------------+------------------------------------------------------------------+
| ``XXX_EXPORT_TEMPLATE_DATA`` | exporting templated data blocks |
+----------------------------------+------------------------------------------------------------------+
| ``XXX_EXPORT_ENUM`` | exporting enum types |
+----------------------------------+------------------------------------------------------------------+

For a new library, the preference might be to call ``XXX_EXPORT``
something like ``XXX_FUNC``, and rename things such as ``XXX_EXPORT_TYPE``
to ``XXX_TYPE`` for simplicity. However, historically, OpenEXR has used
declare all of these, just that they might be needed.

.. list-table::
:header-rows: 1
:align: left

* - macro name
- purpose
* - ``IMATH_EXPORT``
- one of export or import for windows, visibility for others
* - ``IMATH_EXPORT_TYPE``
- for declaring a class / struct as public (for typeinfo / vtable)
* - ``IMATH_HIDDEN``
- used to explicitly hide, especially members of types
* - ``IMATH_EXPORT_TEMPLATE_TYPE``
- stating the template type should be visible
* - ``IMATH_EXPORT_EXTERN_TEMPLATE``
- exporting template types (i.e. extern side of extern template)
* - ``IMATH_EXPORT_TEMPLATE_INSTANCE``
- exporting specific template instantiations (in cpp code)
* - ``IMATH_EXPORT_TEMPLATE_DATA``
- exporting templated data blocks
* - ``IMATH_EXPORT_ENUM``
- exporting enum types

The preference might be to call ``IMATH_EXPORT`` something like
``IMATH_FUNC``, and rename things such as ``IMATH_EXPORT_TYPE`` to
``IMATH_TYPE`` for simplicity. However, historically, OpenEXR has used
the ``_EXPORT`` tag, and so that is preserved for consistency.

References
==========
---------

* LLVM libc++ visibility macros: https://libcxx.llvm.org/docs/DesignDocs/VisibilityMacros.html

Expand Down
8 changes: 4 additions & 4 deletions docs/index.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
.. _Overview:

Overview
########
Imath
#####

.. toctree::
:caption: Overview
:caption: Imath
:maxdepth: 1

Imath is a basic, light-weight, and efficient C++ representation of 2D
Expand Down Expand Up @@ -50,7 +50,7 @@ Community

* **Contribute a Fix, Feature, or Improvement:**

- Read the `Contribution guidelines
- Read the `Contribution Guidelines
<https://github.com/AcademySoftwareFoundation/openexr/blob/main/CONTRIBUTING.md>`_
and `Code of Conduct <https://github.com/AcademySoftwareFoundation/openexr/blob/main/CODE_OF_CONDUCT.md>`_

Expand Down

0 comments on commit 7b8547c

Please sign in to comment.