[Cython] Fwd: Question about how best require compiler options for C sources

[Erik Bray]

On Mon, Apr 11, 2016 at 7:49 PM, Ian Henriksen
<insertinterestingnamehere at gmail.com> wrote:
> That aside, I agree with Nathaniel that exporting public declarations as a
> part of the
> shared object interface was a design mistake. That aside, however, using an
> api
> declaration lets you get equivalent results without exposing anything as a
> part of
> the shared object API. Here's how this all works:

I don't know if I would outright call it a "mistake", but in
retrospect I think we're all in agreement now that maybe the behavior
of "public" should be curtailed?  Or should we still try to keep its
current behavior and "fix" it as I've been trying to do?

> public declarations: export things to C/C++ through the shared object
> interface.
> Provide a header that exports this interface.
> api declarations: export things to C/C++ through capsule objects. Provide a
> header
> for the Python module that exports that interface.
> cimports: Use capsule objects and parsing of pxd files to share things like
> external
> declarations, header includes, inline Cython functions, and Cython functions
> exported by modules between Cython modules.
>
> The public and api use cases are essentially the same most of the time, but
> api
> declarations use capsules rather than the OS's linker.
>
> There are still some trade-offs between public and api functions.
> Technically, the
> api functions require that the initialization routine exported in the api
> header be
> called for each translation unit that uses them. The public api just
> requires that the
> module already be initialized. In cases where no Python functionality is
> used in a
> public function, you may be able to get away with using the function without
> initializing the module, though I really wouldn't recommend that.

Yes, this is the main problem I have with the "api" declaration.
Don't get me wrong, it's a very clever approach and I think for a lot
of cases it's the right thing to do.  But it absolutely requires and
assumes initialization of the module.  In a general case that would be
the right thing to do. But in theory one could have some plain C code
that does not use any of the Python API, and that one can be sure
doesn't require anything initialized by the module init function.

> There are some more subtle issues here though. The reason api functions need
> to
> be initialized on a per-translation unit basis is that things exported as
> api
> declarations are exported as translation-unit-local (static) function
> pointers. They
> aren't shared by the different translation units within a module built from
> multiple
> source files. I think that's a mistake. It'd be ideal if we could have api
> interfaces (or
> something like them) provide things with shared object local visibility
> rather than
> translation unit local visibility. This would require that the API headers
> have more
> carefully structured ifdef directives so that a macro could be set in a
> given
> translation unit to designate when to emit the actual declarations for the
> needed
> pointers rather than just forward declaring them. It would also require that
> the main
> generated c/cpp file define the pointers it uses as shared-object-local
> rather rather
> than static.

Yep.  This is a problem cysignals is having if I recall correctly.  It
would be good to be able to have it both ways.

> In dynd-python we currently solve this problem by defining shared object
> local
> wrappers for the api exported function pointers and then using those
> instead, but
> I'm not a huge fan of that approach. It works well, but results in another
> unnecessary layer of indirection through the source files to connect the C++
> code
> back to its Python bindings.

Yes, I think some kind of context-dependent declarations using
carefully crafted preprocessor directives, or just using separate
header files entirely for the two contexts, would be best.

> With regards to the dllexporting/dllimporting of things: given that public
> declarations
> are already designed to export things through the shared object interface,
> we may
> as well fix the current setup to export the right things. It's a bad design
> that
> probably ought to be deprecated or at least documented better so people know
> not
> to use it unless their case actually requires sidestepping best practices.
> On the
> other hand, it's also a supported interface, so there's value in "fixing"
> it.
>
> I think the best way to do that is the following:
> - mark public symbols as dllimport unless a given (module specific)
> preprocessor
> define is set.
> - people using the public header outside of the module exporting the symbols
> should not have to set the define at all.
> - people using the public header to compile other source files that are
> linked in to
> the same Python module should set the preprocessor flag for that module.

That's the approach I was trying to take originally, yes.  But I think
it's a burden to require developers to set that preprocessor
flag--instead it should happen automatically, (or not at all, with
preference instead for using a completely separate header for
intra-module use).

> On top of that, at some point we still need to fix our api and public
> headers so that
> they still work when included into the translation unit for the main
> Cython-generated
> c/cpp file. This use-case should just forward-declare everything since the
> needed
> symbols are all defined later on in the Cython module. Since static
> variables
> cannot be forward declared in C, this will require that api declarations use
> shared
> object local symbols or that the main generated c/cpp file use some ifdef
> guards
> when it initializes the various pointers in question.

Sounds good--I see no problem with this.

> As far as making an additional header goes, I personally prefer the extra
> preprocessor define. On the other hand, if people think an additional header
> is
> easier to use, then why not make it do something like
>
> #define USE_DLLEXPORT_NOT_DLLIMPORT_FOR_PARTICULAR_MODULE
> #include <the_original_public_header_with_improved_defines.h>
>
> I think, that'd cover all the use cases better.

That would be fine too and reduce duplication.  I think such a header
should still be generated though and it should be documented when to
use which header.

> Anyway, sorry for the length of my remarks on this. There are several issues
> here
> that have been bothering me for quite some time.

No, not at all.  I'm glad I brought it up--it's been productive :)

Best,
Erik