Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added possibility to compile this library as a static or dynamic library #324

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added possibility to compile this library as a static or dynamic library #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ endif
# you want to leave that flag out on production servers).
#

COMPILER_FLAGS = -Wall -c -std=c++11 -fvisibility=hidden -DBUILDING_PHPCPP -Wno-write-strings -MD
COMPILER_FLAGS = -Wall -c -std=c++11 -fvisibility=hidden -DBUILDING_PHPCPP -DBUILDING_SHARED -Wno-write-strings -MD
SHARED_COMPILER_FLAGS = -fpic
STATIC_COMPILER_FLAGS =
PHP_COMPILER_FLAGS = ${COMPILER_FLAGS} `${PHP_CONFIG} --includes`
Expand Down
10 changes: 5 additions & 5 deletions include/streams.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ namespace Php {
/**
* Define the out and err objects
*/
extern thread_local PHPCPP_EXPORT std::ostream out;
extern thread_local PHPCPP_EXPORT std::ostream error;
extern thread_local PHPCPP_EXPORT std::ostream notice;
extern thread_local PHPCPP_EXPORT std::ostream warning;
extern thread_local PHPCPP_EXPORT std::ostream deprecated;
extern thread_local std::ostream out;
extern thread_local std::ostream error;
extern thread_local std::ostream notice;
extern thread_local std::ostream warning;
extern thread_local std::ostream deprecated;
Copy link

Choose a reason for hiding this comment

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

IMO, this change is wrong. It needs a profound solution for non GNU compatible compiler. Basically, it still needs a DSO export, but should encapsulate thread safe pieces. I'd allow myself also to reference #90 (comment) . Anyway, it doesn't belong to the PR about static enablement, as it would be controlled by the header where PHPCPP_EXPORT would evaluate to empty already. Right now, the DLL build is simply broken, anyway.

Thanks.


/**
* End namespace
Expand Down
8 changes: 4 additions & 4 deletions include/value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1197,8 +1197,8 @@ class PHPCPP_EXPORT Value : private HashParent
/**
* Functions that need access to the privates
*/
friend Value constant(const char *name, size_t size);
friend bool define(const char *name, size_t size, const Value &value);
friend PHPCPP_EXPORT Value constant(const char *name, size_t size);
friend PHPCPP_EXPORT bool define(const char *name, size_t size, const Value &value);
Copy link

@weltling weltling Jul 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This and other items from the file should be a separate PR. The issue will show up with any build with the strict visibility flags, fe on Fedora.

Thanks.


/**
* The Globals and Member classes can access the zval directly
Expand All @@ -1220,8 +1220,8 @@ class PHPCPP_EXPORT Value : private HashParent
/**
* Friend functions which have to access that zval directly
*/
friend Value set_exception_handler(const std::function<Value(Parameters &params)> &handler);
friend Value set_error_handler(const std::function<Value(Parameters &params)> &handler, Error error);
friend Value PHPCPP_EXPORT set_exception_handler(const std::function<Value(Parameters &params)> &handler);
friend Value PHPCPP_EXPORT set_error_handler(const std::function<Value(Parameters &params)> &handler, Error error);
};

/**
Expand Down
33 changes: 24 additions & 9 deletions include/visibility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,33 @@

#if defined _WIN32 || defined __CYGWIN__
#ifdef BUILDING_PHPCPP
#ifdef __GNUC__
#define PHPCPP_EXPORT __attribute__ ((dllexport))
#else
#define PHPCPP_EXPORT __declspec(dllexport) // Note: actually gcc seems to also supports this syntax.
#ifdef BUILDING_SHARED
#ifdef __GNUC__
#define PHPCPP_EXPORT __attribute__ ((dllexport))
#else
#define PHPCPP_EXPORT __declspec(dllexport) // Note: actually gcc seems to also supports this syntax.
#endif
#else /* build static library */
#define PHPCPP_EXPORT
#endif
#define DLL_EXPORT
Copy link

Choose a reason for hiding this comment

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

Why does it need two different specifiers? Either PHPCPP_EXPORT targets DLL or static build.

Thanks.

#else
#ifdef __GNUC__
#define DLL_EXPORT __attribute__ ((dllimport))
#else
#define DLL_EXPORT __declspec(dllimport) // Note: actually gcc seems to also supports this syntax.
#ifdef BUILDING_SHARED
#ifdef __GNUC__
#define DLL_EXPORT __attribute__ ((dllimport))
#else
#define DLL_EXPORT __declspec(dllimport) // Note: actually gcc seems to also supports this syntax.
#endif
#else /* build static library */
#define DLL_EXPORT
#endif
#define PHPCPP_EXPORT
#endif
#else
#define PHPCPP_EXPORT __attribute__ ((visibility ("default")))
#ifdef BUILDING_SHARED
#define PHPCPP_EXPORT __attribute__ ((visibility ("default")))
#else /* build static library */
#define PHPCPP_EXPORT
#endif
#define DLL_EXPORT
Copy link

Choose a reason for hiding this comment

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

This piece seems wrong. If the attribute is supported, DLL_EXPORT should be same as PHPCPP_EXPORT.

In general I see what you're striving to achieve, but it is not going to work well. An extension should define its own API inside it, using same specifier will still require additional compiler flags. Fe if there's a static phpcpp lib, get_module() inside the extension will still have to be DLL exported.

Thanks.

#endif