Skip to content

Make OfxExport export a symbol on GCC and Clang, and use it in the examples - #274

Merged
garyo merged 4 commits into
mainfrom
fix/ofxexport-visibility
Sep 23, 2026
Merged

garyo merged 4 commits into
mainfrom
fix/ofxexport-visibility

Conversation

@garyo

@garyo garyo commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • OfxExport expanded to __declspec(dllexport) on Windows and to plain extern everywhere else, so on macOS and Linux it did nothing. A plugin built with hidden visibility (as the CMake build does, via CXX_VISIBILITY_PRESET hidden) exported no entry points through it, and every example carried its own eight-line EXPORT block to supply the visibility attribute.
  • OfxExport now adds __attribute__((visibility("default"))) under GCC and Clang. The entry points OfxGetPlugin, OfxGetNumberOfPlugins and OfxSetHost are declared with it in ofxCore.h, and a plugin's definitions inherit the attribute from those declarations, so they are exported with no specifier of their own.
  • The examples, and the Programming Guide's five example plugins under Documentation/sources/Guide/Code/, drop their EXPORT macros and define the entry points with no specifier, the pattern plugin authors can copy (18 files, net -122 lines). That also matches the guide's own snippets, which never showed a specifier. Support/Library keeps its own EXPORT/LOCAL pair for now, since it also uses LOCAL.
  • The ColourSpace example's OfxSetHost took no parameter. In C++ that made it a separate overload with C++ linkage, exported under a mangled name (__Z10OfxSetHostv), so no host ever found or called it. It now takes the const OfxHost * the header declares.

Compatibility

  • Plugins that already put OfxExport or their own visibility attribute on their definitions keep working: the attribute is simply repeated, which compilers accept silently.

Test plan

  • Every example the CMake build produces (hidden visibility) exports exactly OfxGetPlugin and OfxGetNumberOfPlugins, plus OfxSetHost for ColourSpace, on macOS (checked with nm -gU). Overlay, which is not in the CMake plugin list, and the five guide examples checked the same way by hand; all build warning-free.
  • Control: bare definitions of the three entry points against main's ofxCore.h export nothing under -fvisibility=hidden; against this branch they export exactly those three, in both C and C++, and an undeclared helper stays hidden.
  • CI on Linux and Windows.

Assisted-by: Claude Code / Claude Fable 5.1, Claude Opus 5.5

…amples

OfxExport expanded to __declspec(dllexport) on Windows and to plain
extern everywhere else, so on macOS and Linux it did nothing. A plugin
built with hidden visibility, as the CMake and pcons builds do, exported
no entry points through it, and every example carried its own eight-line
EXPORT block to add the visibility attribute. OfxExport now adds
__attribute__((visibility("default"))) under GCC and Clang, which is what
the header's own OfxGetPlugin and OfxGetNumberOfPlugins declarations need.

The examples drop their EXPORT macros and use OfxExport directly. Each
still exports exactly OfxGetPlugin and OfxGetNumberOfPlugins when built
with -fvisibility=hidden.

Assisted-by: Claude Code / Claude Fable 5.1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
@garyo garyo added the fix label Sep 22, 2026
@garyo garyo added this to the Release 1.6 milestone Sep 22, 2026
@garyo
garyo marked this pull request as ready for review September 22, 2026 19:12

@Guido-assim Guido-assim left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function declarations for OfxGetPlugin, OfxGetNumberOfPlugins and OfxSetHost in ofxCore.h already have the OfxExport specifier, I don't think it is needed to have it again with the definitions in the cpp files.
The definition of OfxSetHost in colourspace.cpp seems to be missing the host argument.

…Host

With the visibility attribute on the entry-point declarations in
ofxCore.h, a plugin's definitions inherit it: built with hidden
visibility, bare definitions of OfxGetPlugin, OfxGetNumberOfPlugins and
OfxSetHost are exported and nothing else is. The examples now define
them with no specifier, which is the pattern plugin authors should
copy.

The ColourSpace example's OfxSetHost took no parameter. In C++ that is
a separate overload with C++ linkage, exported under a mangled name, so
no host ever found or called it; it now takes the const OfxHost * the
header declares.

The release note no longer mentions a pcons build, which main does not
have.

Assisted-by: Claude Code / Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
@garyo

garyo commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

Good catch on both, Guido. Updated the PR to remove the decls in the examples. Also I fixed the colourspace example (that was preventing OfxSetHost from getting called on that example altogether, since it's a different function, not even "export "C"").

The Programming Guide's five example plugins each defined the same
EXPORT macro as the examples did, and marked their entry points with
it. The definitions now inherit OfxExport from the declarations in
ofxCore.h, which also matches the snippets the guide shows, which never
had a specifier. Each still builds warning-free with hidden visibility
and exports exactly OfxGetPlugin and OfxGetNumberOfPlugins.

Assisted-by: Claude Code / Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>

@Guido-assim Guido-assim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@Guido-assim Guido-assim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@garyo
garyo merged commit 4067c7d into main Sep 23, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants