[
https://issues.apache.org/jira/browse/LUCY-231?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13259203#comment-13259203
]
Nick Wellnhofer commented on LUCY-231:
--------------------------------------
> 0003-Unify-method-data-for-initialization-and-callbacks.patch looks good and
> suggests an interesting direction outside the scope of this issue, which I
> will post about on the dev list. FWIW, I'd suggest "cfish_Method" for the
> name of the struct, or possibly "cfish_MetaData" or "cfish_MetaMethod", and
> "_META" for the variables.
Hmm, "cfish_Method" would clash with the name of a real Method class once it's
introduced. Although we could create Method objects without using the struct
then, it might be worth to keep the struct for initialization. It's more
compact memory-wise than to unroll everything into function arguments.
"cfish_MetaData" doesn't tell that the metadata is about methods.
> 0005-Switch-to-fvisibility-hidden-and-start-using-CHY_EXP.patch looks good,
> thought I have one comment. It took me a while to figure out the reasoning
> behind changing up the export symbols in the autogenerated .h files based on
> whether the .cfh file was "included". Up until now the output of CFC has
> been deterministic for a given set of .cfh files; after this patch, that's
> no longer true. My first instinct would have been to use an #ifdef chain as
> described above; I think you chose to do things this way so that the
> extension author wouldn't have to add an extra pound-define to to their C
> files, right?
We have to exchange CHY_EXPORT for CHY_IMPORT in headers of included classes
because of the way DLL symbols are handled on Windows.
The reason I went with my approach is that we'd have to introduce per-parcel
#defines if we want to make the CFC output deterministic. That's more
complicated. It would also mean that an extension could never use the same
parcel as the project it extends.
If we use the #ifdef trick, the question is where to put that #ifdef. I think
it should go in a per-parcel header that gets installed in Clownfish/_include,
but we don't have something like that at the moment.
We could also autogenerate a #define in parcel.h for every parcel. In Lucy we'd
have for example:
{noformat}
#define LUCY_PUBLIC CHY_EXPORT
{noformat}
In an extension it would be:
{noformat}
#define LUCY_PUBLIC CHY_IMPORT
#define EXT_PUBLIC CHY_EXPORT
{noformat}
This would mean to keep track of every parcel encountered during compilation.
It would also make the contents of parcel.h non-deterministic again.
> Symbol visibility
> -----------------
>
> Key: LUCY-231
> URL: https://issues.apache.org/jira/browse/LUCY-231
> Project: Lucy
> Issue Type: Task
> Components: Clownfish
> Reporter: Nick Wellnhofer
> Assignee: Nick Wellnhofer
> Attachments:
> 0001-Preliminary-Charmonizer-support-for-symbol-export.patch,
> 0002-Switch-to-fvisibility-hidden-and-start-using-CHY_EXP.patch,
> 0003-Unify-method-data-for-initialization-and-callbacks.patch,
> 0004-Preliminary-Charmonizer-support-for-symbol-export.patch,
> 0005-Switch-to-fvisibility-hidden-and-start-using-CHY_EXP.patch,
> 0006-Install-import-library-on-Windows.patch
>
>
> In order to get compiled extension working on Windows, we'll have to define
> the visibility of our extern variables and functions. On Windows every
> exported symbol of a DLL has to be marked with __declspec(dllexport) when
> compiling the DLL. If you're linking against a DLL, every symbol imported
> from the DLL has to be marked __declspec(dllimport)
> On UNIX, every symbol is exported by default, so defining visibility is not
> strictly necessary. But hiding symbols that don't have to be exported has the
> benefit of reducing size and speeding up loading of a DLL. Hidden symbols
> also allow the compiler to generate more optimized code.
> The standard approach is to compile with the GCC option -fvisibility=hidden
> which emulates the Windows behavior. Then a macro is defined roughly like
> that (should probably be handled by charmonizer):
> {noformat}
> #if defined __GNUC__
> # if defined _WIN32 || defined __CYGWIN__
> # define CHY_EXPORT __attribute__ ((dllexport))
> # elif __GNUC__ >= 4
> # define CHY_EXPORT __attribute__ ((visibility ("default")))
> # else
> # define CHY_EXPORT
> # endif
> #elif defined _MFC_VER
> # define CHY_EXPORT __declspec(dllexport)
> #else
> # define CHY_EXPORT
> #endif
> {noformat}
> This macro can then be used like that:
> {noformat}
> CHY_EXPORT void
> exported_function();
> extern CHY_EXPORT int exported_variable;
> {noformat}
> When compiling an extension, we also have to handle __declspec(dllimport) on
> Windows. For the generated headers, we could define CHY_IMPORT and use it
> instead of CHY_EXPORT for "included" headers. For the code in XSBind.[ch], we
> could define BUILDING_XSBIND only during compilation of XSBind.c and then use
> something like that:
> {noformat}
> #if defined __GNUC__
> # if defined _WIN32 || defined __CYGWIN__
> # if BUILDING_XSBIND
> # define XSBIND_EXPORT __attribute__ ((dllexport))
> # else
> # define XSBIND_EXPORT __attribute__ ((dllimport))
> # endif
> # elif __GNUC__ >= 4
> # define XSBIND_EXPORT __attribute__ ((visibility ("default")))
> # else
> # define XSBIND_EXPORT
> # endif
> #elif defined _MFC_VER
> # if BUILDING_XSBIND
> # define XSBIND_EXPORT __declspec(dllexport)
> # else
> # define XSBIND_EXPORT __declspec(dllimport)
> # endif
> #else
> # define XSBIND_EXPORT
> #endif
> XSBIND_EXPORT cfish_Obj*
> cfish_XSBind_new_blank_obj(SV *either_sv);
> {noformat}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
