You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/08/21 08:38:41 UTC

[GitHub] [logging-log4cxx] swebb2066 opened a new pull request, #117: Prevent MSVC 'nedds to have a dll interface' warnings

swebb2066 opened a new pull request, #117:
URL: https://github.com/apache/logging-log4cxx/pull/117

   This commit is progrees toward resolving [LOGCXX-558](https://issues.apache.org/jira/browse/LOGCXX-558) 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4cxx] swebb2066 commented on a diff in pull request #117: Prevent MSVC 'needs to have a dll interface' warnings

Posted by GitBox <gi...@apache.org>.
swebb2066 commented on code in PR #117:
URL: https://github.com/apache/logging-log4cxx/pull/117#discussion_r962070354


##########
src/main/include/log4cxx/log4cxx.h.in:
##########
@@ -76,8 +83,11 @@ typedef unsigned int log4cxx_uint32_t;
 //    definitions used when using DLL
 #define LOG4CXX_EXPORT __declspec(dllimport)
 #endif
+#define LOG4CXX_INSTANTIATE_EXPORTED_PTR(T) template class LOG4CXX_EXPORT std::shared_ptr<T>
 #else
 #define LOG4CXX_EXPORT
+#define LOG4CXX_DECLARE_PRIVATE_MEMBER_PTR(T, V) struct T; LOG4CXX_PRIVATE_PTR(T) V;
+#define LOG4CXX_INSTANTIATE_EXPORTED_PTR(T)

Review Comment:
   gcc says instantiated templates must be in their own namespace (not sure how to interpret that when the template and the template parameter are in different namespaces). Visual Studio does not issue that error, but not instantiating the template at all results in a  'needs to have a dll interface' warning.
   
   My solution was to only instantiate when using Visual Studio.
   
   See for example, in propertysetter.h



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4cxx] swebb2066 merged pull request #117: Prevent MSVC 'needs to have a dll interface' warnings

Posted by GitBox <gi...@apache.org>.
swebb2066 merged PR #117:
URL: https://github.com/apache/logging-log4cxx/pull/117


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4cxx] rm5248 commented on a diff in pull request #117: Prevent MSVC 'needs to have a dll interface' warnings

Posted by GitBox <gi...@apache.org>.
rm5248 commented on code in PR #117:
URL: https://github.com/apache/logging-log4cxx/pull/117#discussion_r962224217


##########
src/main/include/log4cxx/log4cxx.h.in:
##########
@@ -64,8 +64,15 @@ typedef unsigned int log4cxx_uint32_t;
 	typedef std::weak_ptr<T> T##WeakPtr
 #define LOG4CXX_UNIQUE_PTR_DEF(T) typedef std::unique_ptr<T> T##UniquePtr;
 #define LOG4CXX_LIST_DEF(N, T) typedef std::vector<T> N
+#define LOG4CXX_PRIVATE_PTR(T) std::unique_ptr<T>

Review Comment:
   It feels like it doesn't really add anything in terms of making things simpler to read(to me).
   
   Not really a huge issue, more of a preference thing in how to organize the code.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4cxx] swebb2066 commented on a diff in pull request #117: Prevent MSVC 'needs to have a dll interface' warnings

Posted by GitBox <gi...@apache.org>.
swebb2066 commented on code in PR #117:
URL: https://github.com/apache/logging-log4cxx/pull/117#discussion_r962069230


##########
src/main/include/log4cxx/log4cxx.h.in:
##########
@@ -64,8 +64,15 @@ typedef unsigned int log4cxx_uint32_t;
 	typedef std::weak_ptr<T> T##WeakPtr
 #define LOG4CXX_UNIQUE_PTR_DEF(T) typedef std::unique_ptr<T> T##UniquePtr;
 #define LOG4CXX_LIST_DEF(N, T) typedef std::vector<T> N
+#define LOG4CXX_PRIVATE_PTR(T) std::unique_ptr<T>

Review Comment:
   I only wanted the definitions to be in the same place, so the usage is (for example)
   		LOG4CXX_DECLARE_PRIVATE_MEMBER_PTR(AppenderSkeletonPrivate, m_priv)
   		AppenderSkeleton(LOG4CXX_PRIVATE_PTR(AppenderSkeletonPrivate) priv);
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4cxx] rm5248 commented on a diff in pull request #117: Prevent MSVC 'needs to have a dll interface' warnings

Posted by GitBox <gi...@apache.org>.
rm5248 commented on code in PR #117:
URL: https://github.com/apache/logging-log4cxx/pull/117#discussion_r952056193


##########
src/main/include/log4cxx/log4cxx.h.in:
##########
@@ -76,8 +83,11 @@ typedef unsigned int log4cxx_uint32_t;
 //    definitions used when using DLL
 #define LOG4CXX_EXPORT __declspec(dllimport)
 #endif
+#define LOG4CXX_INSTANTIATE_EXPORTED_PTR(T) template class LOG4CXX_EXPORT std::shared_ptr<T>
 #else
 #define LOG4CXX_EXPORT
+#define LOG4CXX_DECLARE_PRIVATE_MEMBER_PTR(T, V) struct T; LOG4CXX_PRIVATE_PTR(T) V;
+#define LOG4CXX_INSTANTIATE_EXPORTED_PTR(T)

Review Comment:
   Do we need to have two versions of this macro?  I don't know enough about how windows works, but would it be possible to just add `LOG4CXX_EXPORT` to the already-existing `LOG4CXX_PTR_DEF` macro?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4cxx] rm5248 commented on a diff in pull request #117: Prevent MSVC 'needs to have a dll interface' warnings

Posted by GitBox <gi...@apache.org>.
rm5248 commented on code in PR #117:
URL: https://github.com/apache/logging-log4cxx/pull/117#discussion_r952054695


##########
src/main/include/log4cxx/log4cxx.h.in:
##########
@@ -64,8 +64,15 @@ typedef unsigned int log4cxx_uint32_t;
 	typedef std::weak_ptr<T> T##WeakPtr
 #define LOG4CXX_UNIQUE_PTR_DEF(T) typedef std::unique_ptr<T> T##UniquePtr;
 #define LOG4CXX_LIST_DEF(N, T) typedef std::vector<T> N
+#define LOG4CXX_PRIVATE_PTR(T) std::unique_ptr<T>

Review Comment:
   Is this define really needed?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org