[jira] [Commented] (GEODE-9559) Demacroize clicache
[ https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17414458#comment-17414458 ] ASF GitHub Bot commented on GEODE-9559: --- mmartell closed pull request #862: URL: https://github.com/apache/geode-native/pull/862 -- 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-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Demacroize clicache > > > Key: GEODE-9559 > URL: https://issues.apache.org/jira/browse/GEODE-9559 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Martell >Priority: Major > Labels: pull-request-available > > Macros in C++ complicate debug efforts and code maintenance and are generally > considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This > PR is to remove all the complicated macros in the .NET Framework client, e.g. > the clicache module. > In addition to improving the maintainability of the clicache module, removing > the macros will greatly assist the creation of the .NET Core client. [dotPeek > |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the > .NET Core project, but is currently limited by the extensive use of macros in > the clicache code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[ https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17414457#comment-17414457 ] ASF GitHub Bot commented on GEODE-9559: --- mmartell commented on pull request #862: URL: https://github.com/apache/geode-native/pull/862#issuecomment-918441488 Closing this PR since it doesn't add new functionality or fix an existing bug. For anyone interested in decompiling the C++/CLI code they can checkout my branch: mmartell:GEODE-9559-demacroize-clicache -- 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-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Demacroize clicache > > > Key: GEODE-9559 > URL: https://issues.apache.org/jira/browse/GEODE-9559 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Martell >Priority: Major > Labels: pull-request-available > > Macros in C++ complicate debug efforts and code maintenance and are generally > considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This > PR is to remove all the complicated macros in the .NET Framework client, e.g. > the clicache module. > In addition to improving the maintainability of the clicache module, removing > the macros will greatly assist the creation of the .NET Core client. [dotPeek > |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the > .NET Core project, but is currently limited by the extensive use of macros in > the clicache code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[
https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17409212#comment-17409212
]
ASF GitHub Bot commented on GEODE-9559:
---
mmartell commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r701534899
##
File path: clicache/src/CacheTransactionManager.cpp
##
@@ -33,111 +33,165 @@ namespace Apache
void CacheTransactionManager::Begin( )
{
-_GF_MG_EXCEPTION_TRY2
-
+try {
m_nativeptr->begin( );
-
-_GF_MG_EXCEPTION_CATCH_ALL2
+}
+catch (const apache::geode::client::Exception& ex) {
+ throw Apache::Geode::Client::GeodeException::Get(ex);
+}
+catch (System::AccessViolationException^ ex) {
+ throw ex;
+}
}
void CacheTransactionManager::Prepare( )
{
-_GF_MG_EXCEPTION_TRY2
-
+try {
Review comment:
We don't currently use any lambdas in C++/CLI code. It's not clear to me
if it is even supported. Possibly for unmanaged code I believe you are
indicating. However, for this PR I'm not sure it's with introducing new
features in a deprecated compiler.
--
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-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Demacroize clicache
>
>
> Key: GEODE-9559
> URL: https://issues.apache.org/jira/browse/GEODE-9559
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Michael Martell
>Priority: Major
> Labels: pull-request-available
>
> Macros in C++ complicate debug efforts and code maintenance and are generally
> considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This
> PR is to remove all the complicated macros in the .NET Framework client, e.g.
> the clicache module.
> In addition to improving the maintainability of the clicache module, removing
> the macros will greatly assist the creation of the .NET Core client. [dotPeek
> |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the
> .NET Core project, but is currently limited by the extensive use of macros in
> the clicache code.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[
https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17409189#comment-17409189
]
ASF GitHub Bot commented on GEODE-9559:
---
mmartell commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r701527622
##
File path: clicache/src/Cache.cpp
##
@@ -159,13 +165,19 @@ namespace Apache
GC::KeepAlive(m_nativeptr);
}
-_GF_MG_EXCEPTION_CATCH_ALL2
+}
+catch (const apache::geode::client::Exception& ex) {
+ throw Apache::Geode::Client::GeodeException::Get(ex);
+}
+catch (System::AccessViolationException^ ex) {
+ throw ex;
+}
}
generic
Client::IRegion^ Cache::GetRegion( String^ path )
{
-_GF_MG_EXCEPTION_TRY2/* due to auto replace */
+try {/* due to auto replace */
Review comment:
Done
--
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-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Demacroize clicache
>
>
> Key: GEODE-9559
> URL: https://issues.apache.org/jira/browse/GEODE-9559
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Michael Martell
>Priority: Major
> Labels: pull-request-available
>
> Macros in C++ complicate debug efforts and code maintenance and are generally
> considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This
> PR is to remove all the complicated macros in the .NET Framework client, e.g.
> the clicache module.
> In addition to improving the maintainability of the clicache module, removing
> the macros will greatly assist the creation of the .NET Core client. [dotPeek
> |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the
> .NET Core project, but is currently limited by the extensive use of macros in
> the clicache code.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[
https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17408966#comment-17408966
]
ASF GitHub Bot commented on GEODE-9559:
---
mmartell commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r701247030
##
File path: clicache/src/ExceptionTypes.hpp
##
@@ -266,417 +266,3411 @@ namespace Apache
: Exception( info, context ) { }
};
-/// Handle geode exceptions from native layer and convert to managed
-/// exceptions.
-#define _GF_MG_EXCEPTION_TRY2\
- try {
-#define _GF_MG_EXCEPTION_CATCH_ALL2 \
- } \
- catch (const apache::geode::client::Exception& ex) { \
- throw Apache::Geode::Client::GeodeException::Get(ex); \
- } \
- catch (System::AccessViolationException^ ex) { \
-throw ex; \
- }
-
-
-/// Creates a class x named for each exception y.
-#define _GF_MG_EXCEPTION_DEF4(x,y) \
- [Serializable] \
- public ref class x: public GeodeException \
- { \
- public: \
- \
-/** Default constructor */ \
-x( ) \
- : GeodeException( ) { } \
-\
-/**
- * Constructor to create an exception object with the given message.
- *
- * The exception message.
- */ \
-x( String^ message ) \
- : GeodeException( message ) { } \
-\
-/**
- * Constructor to create an exception object with the given message
- * and with the given inner exception.
- *
- * The exception message.
- * The inner exception object.
- */ \
-x( String^ message, System::Exception^ innerException ) \
- : GeodeException( message, innerException ) { } \
-\
- protected: \
- \
-/**
- * Initializes a new instance of the class with serialized data.
- * This allows deserialization of this exception in .NET remoting.
- *
- *
- * holds the serialized object data about the exception being thrown
- *
- *
- * contains contextual information about the source or destination
- *
- */ \
-x( SerializationInfo^ info, StreamingContext context ) \
- : GeodeException( info, context ) { } \
- \
- internal: \
-x(const apache::geode::client::y& nativeEx) \
- : GeodeException(marshal_as(nativeEx.getMessage()), \
- gcnew GeodeException(GeodeException::GetStackTrace( \
-nativeEx))) { } \
-\
-x(const apache::geode::client::y& nativeEx, Exception^ innerException)
\
- : GeodeException(marshal_as(nativeEx.getMessage()), \
- innerException) { } \
-\
-static GeodeException^ Create(const apache::geode::client::Exception&
ex, \
-Exception^ innerException) \
-{ \
- const apache::geode::client::y* nativeEx = dynamic_cast( &ex ); \
- if (nativeEx != nullptr) { \
-if (innerException == nullptr) { \
- return gcnew x(*nativeEx); \
-} \
-else { \
- return gcnew x(*nativeEx, innerException); \
-} \
- } \
- return nullptr; \
-} \
-virtual std::shared_ptr GetNative()
override \
-{ \
- auto message = marshal_as(this->Message + ": " +
this->StackTrace); \
- if (this->InnerException != nullptr) { \
-auto cause = GeodeException::GetNative(this->InnerException); \
-message += "Caused by: " + cause->getMessage(); \
- } \
- return std::make_shared(message); \
-} \
- }
-
-/// Creates a class named for each exception x.
-#define _GF_MG_EXCEPTION_DEF3(x) _GF_MG_EXCEPTION_DEF4(x,x)
-
-
// For all the native Geode C++ exceptions, a corresponding definition
- // should be added below *AND* it should also be added to the static
array
- // in ExceptionTypesMN.cpp using _GF_MG_EXCEPTION_ADD3( x )
-
+ // should be added below.
///
/// A geode assertion exception.
///
- _GF_MG_EXCEPTION_DEF3( AssertionException );
+ [Serializable] public ref class AssertionException
+ : public GeodeException {
+ public:
+AssertionException() : GeodeException() {}
+AssertionException(String ^ message) : GeodeException(message) {}
+AssertionException(String ^ message, System::Exception ^
innerException)
+: GeodeException(message, innerException) {}
+
+ protected:
+AssertionException(SerializationInfo ^ info, StreamingContext context)
+: GeodeException(info, context) {}
+ internal:
+AssertionException(
+ const apache::geode::client::AssertionException
[jira] [Commented] (GEODE-9559) Demacroize clicache
[ https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17408918#comment-17408918 ] ASF GitHub Bot commented on GEODE-9559: --- mmartell commented on pull request #862: URL: https://github.com/apache/geode-native/pull/862#issuecomment-911790472 Although Microsoft has a new preprocessor that follows the standard (I didn't even know there was a standard), it doesn't appear to allow enabling/disabling specific macro expansion. However, I'm not too worried about copy/paste errors here. The compiler and subsequent clicache tests should catch any errors. -- 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-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Demacroize clicache > > > Key: GEODE-9559 > URL: https://issues.apache.org/jira/browse/GEODE-9559 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Martell >Priority: Major > Labels: pull-request-available > > Macros in C++ complicate debug efforts and code maintenance and are generally > considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This > PR is to remove all the complicated macros in the .NET Framework client, e.g. > the clicache module. > In addition to improving the maintainability of the clicache module, removing > the macros will greatly assist the creation of the .NET Core client. [dotPeek > |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the > .NET Core project, but is currently limited by the extensive use of macros in > the clicache code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[
https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17408915#comment-17408915
]
ASF GitHub Bot commented on GEODE-9559:
---
mmartell commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r701172868
##
File path: clicache/src/ExceptionTypes.hpp
##
@@ -266,417 +266,3411 @@ namespace Apache
: Exception( info, context ) { }
};
-/// Handle geode exceptions from native layer and convert to managed
-/// exceptions.
-#define _GF_MG_EXCEPTION_TRY2\
- try {
-#define _GF_MG_EXCEPTION_CATCH_ALL2 \
- } \
- catch (const apache::geode::client::Exception& ex) { \
- throw Apache::Geode::Client::GeodeException::Get(ex); \
- } \
- catch (System::AccessViolationException^ ex) { \
-throw ex; \
- }
-
-
-/// Creates a class x named for each exception y.
-#define _GF_MG_EXCEPTION_DEF4(x,y) \
- [Serializable] \
- public ref class x: public GeodeException \
- { \
- public: \
- \
-/** Default constructor */ \
-x( ) \
- : GeodeException( ) { } \
-\
-/**
- * Constructor to create an exception object with the given message.
- *
- * The exception message.
- */ \
-x( String^ message ) \
- : GeodeException( message ) { } \
-\
-/**
- * Constructor to create an exception object with the given message
- * and with the given inner exception.
- *
- * The exception message.
- * The inner exception object.
- */ \
-x( String^ message, System::Exception^ innerException ) \
- : GeodeException( message, innerException ) { } \
-\
- protected: \
- \
-/**
- * Initializes a new instance of the class with serialized data.
- * This allows deserialization of this exception in .NET remoting.
- *
- *
- * holds the serialized object data about the exception being thrown
- *
- *
- * contains contextual information about the source or destination
- *
- */ \
-x( SerializationInfo^ info, StreamingContext context ) \
- : GeodeException( info, context ) { } \
- \
- internal: \
-x(const apache::geode::client::y& nativeEx) \
- : GeodeException(marshal_as(nativeEx.getMessage()), \
- gcnew GeodeException(GeodeException::GetStackTrace( \
-nativeEx))) { } \
-\
-x(const apache::geode::client::y& nativeEx, Exception^ innerException)
\
- : GeodeException(marshal_as(nativeEx.getMessage()), \
- innerException) { } \
-\
-static GeodeException^ Create(const apache::geode::client::Exception&
ex, \
-Exception^ innerException) \
-{ \
- const apache::geode::client::y* nativeEx = dynamic_cast( &ex ); \
- if (nativeEx != nullptr) { \
-if (innerException == nullptr) { \
- return gcnew x(*nativeEx); \
-} \
-else { \
- return gcnew x(*nativeEx, innerException); \
-} \
- } \
- return nullptr; \
-} \
-virtual std::shared_ptr GetNative()
override \
-{ \
- auto message = marshal_as(this->Message + ": " +
this->StackTrace); \
- if (this->InnerException != nullptr) { \
-auto cause = GeodeException::GetNative(this->InnerException); \
-message += "Caused by: " + cause->getMessage(); \
- } \
- return std::make_shared(message); \
-} \
- }
-
-/// Creates a class named for each exception x.
-#define _GF_MG_EXCEPTION_DEF3(x) _GF_MG_EXCEPTION_DEF4(x,x)
-
-
// For all the native Geode C++ exceptions, a corresponding definition
- // should be added below *AND* it should also be added to the static
array
- // in ExceptionTypesMN.cpp using _GF_MG_EXCEPTION_ADD3( x )
-
+ // should be added below.
///
/// A geode assertion exception.
///
- _GF_MG_EXCEPTION_DEF3( AssertionException );
+ [Serializable] public ref class AssertionException
+ : public GeodeException {
+ public:
+AssertionException() : GeodeException() {}
+AssertionException(String ^ message) : GeodeException(message) {}
+AssertionException(String ^ message, System::Exception ^
innerException)
+: GeodeException(message, innerException) {}
+
+ protected:
+AssertionException(SerializationInfo ^ info, StreamingContext context)
+: GeodeException(info, context) {}
+ internal:
+AssertionException(
+ const apache::geode::client::AssertionException
[jira] [Commented] (GEODE-9559) Demacroize clicache
[
https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17408912#comment-17408912
]
ASF GitHub Bot commented on GEODE-9559:
---
mmartell commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r701168532
##
File path: clicache/src/CacheableHashSet.hpp
##
@@ -598,69 +604,11 @@ namespace Apache
};
}
-#define _GFCLI_CACHEABLEHASHSET_DEF_GENERIC(m, HSTYPE)
\
- public ref class m : public
Internal::CacheableHashSetType(native::internal::DSCode::m),
HSTYPE> \
-{
\
- public: \
-/**
- * Allocates a new empty instance.
- *
- */ \
- inline m()\
- :
Internal::CacheableHashSetType(native::internal::DSCode::m),
HSTYPE>() {} \
- \
- /**
- * Allocates a new instance with the given size.
- *
- * the initial size of the new instance
- */ \
- inline m(System::Int32 size)
\
- :
Internal::CacheableHashSetType(native::internal::DSCode::m),
HSTYPE>(size) {} \
- \
- /**
-* Static function to create a new empty instance.
-*
-*/ \
-inline static m^ Create() \
- { \
- return gcnew m(); \
- } \
- \
- /**
- * Static function to create a new instance with the given size.
- *
- */ \
- inline static m^ Create(System::Int32 size)
\
- { \
- return gcnew m(size); \
- } \
- \
- /*
- * Factory function to register this class.
- *
- */ \
- static ISerializable^ CreateDeserializable()\
- { \
- return gcnew m(); \
- } \
- \
-internal:
\
- static ISerializable^
Create(std::shared_ptr obj)\
- { \
- return gcnew m(obj);\
- } \
- \
-private:
\
- inline m(std::shared_ptr
nativeptr)\
- :
Internal::CacheableHashSetType(native::internal::DSCode::m),
HSTYPE>(nativeptr) { } \
- };
-
///
/// A mutable ICacheableKey hash set wrapper that can serve as
/// a distributable object for caching.
///
- _GFCLI_CACHEABLEHASHSET_DEF_GENERIC(CacheableHashSet,
-
apache::geode::client::CacheableHashSet);
+ public ref class CacheableHashSet : public
Internal::CacheableHashSetType(native::internal::DSCode::CacheableHashSet),
apache::geode::client::CacheableHashSet> { public: inline CacheableHashSet() :
Internal::CacheableHashSetType(native::internal::DSCode::CacheableHashSet),
apache::geode::client::CacheableHashSet>() {} inline
CacheableHashSet(System::Int32 size) :
Internal::CacheableHashSetType(native::internal::DSCode::CacheableHashSet),
apache::geode::client::CacheableHashSet>(size) {} inline static
CacheableHashSet^ Create() { return gcnew CacheableHashSet(); } inline static
CacheableHashSet^ Create(System::Int32 size) { return gcnew
CacheableHashSet(size); } static ISerializable^ CreateDeserializable() { return
gcnew CacheableHashSet(); } internal: static ISerializable^
Create(std::shared_ptr obj) { return gcnew
CacheableHashSet(obj); } private: inl
[jira] [Commented] (GEODE-9559) Demacroize clicache
[
https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17408906#comment-17408906
]
ASF GitHub Bot commented on GEODE-9559:
---
mmartell commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r701161709
##
File path: clicache/src/CacheableBuiltins.hpp
##
@@ -356,129 +356,56 @@ namespace Apache
}
};
-
-
-
- //n = native type
- //m = CacheableInt(managed cacheable)
- //mt = managed type(bool, int)
-#define _GFCLI_CACHEABLE_KEY_DEF_NEW(n, m, mt)
\
- ref class m : public CacheableBuiltinKey(DSCode::m)>
\
- { \
- public: \
- /**
- * Allocates a new instance with the given value.
- *
- * the value of the new instance
- */ \
- inline m()
\
- : CacheableBuiltinKey() { } \
- /**
- * Allocates a new instance with the given value.
- *
- * the value of the new instance
- */
\
- inline m(mt value)
\
- : CacheableBuiltinKey(value) { }\
- /**
- * Static function to create a new instance given value.
- *
- * the value of the new instance
- */
\
- inline static m^ Create(mt value)
\
- {
\
- return gcnew m(value);
\
- }
\
- /**
-* Explicit conversion operator to contained value type.
-*
-*/
\
-inline static explicit operator mt (m^ value)
\
- {
\
- return value->Value;
\
- }
\
- \
- /**
-* Factory function to register this class.
-*
-*/
\
-static ISerializable^ CreateDeserializable()
\
- {
\
- return gcnew m(); \
- }
\
- \
- internal:
\
- static ISerializable^ Create(std::shared_ptr
obj)\
- {
\
- return (obj != nullptr ? gcnew m(obj) : nullptr);
\
- }
\
- \
- private:
\
- inline m(std::shared_ptr nativeptr)
\
- : CacheableBuiltinKey(nativeptr) { }
\
- };
-
-
-
-
// Built-in CacheableKeys
///
/// An immutable wrapper for booleans that can serve
/// as a distributable key object for caching.
///
- _GFCLI_CACHEABLE_KEY_DEF_NEW(native::CacheableBoolean,
- CacheableBoolean, bool);
+ ref class CacheableBoolean : public
CacheableBuiltinKey(DSCode::CacheableBoolean)> { public: inline
CacheableBoolean() : CacheableBuiltinKey() { } inline CacheableBoolean(bool
value) : CacheableBuiltinKey(value) { } inline static CacheableBoolean^
Create(bool value) { return gcnew CacheableBoolean(value); } inline static
explicit operator bool (CacheableBoolean^ value) { return value->Value; }
static ISerializable^ CreateDeserializable() { return gcnew CacheableBoolean();
} internal: static ISerializable^ Create(std::shared_ptr
obj) { return (obj != nullptr ? gcnew CacheableBoolean(obj) : nullptr); }
privat
[jira] [Commented] (GEODE-9559) Demacroize clicache
[
https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17408827#comment-17408827
]
ASF GitHub Bot commented on GEODE-9559:
---
mmartell commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r701088758
##
File path: clicache/src/CacheTransactionManager.cpp
##
@@ -33,111 +33,165 @@ namespace Apache
void CacheTransactionManager::Begin( )
{
-_GF_MG_EXCEPTION_TRY2
-
+try {
m_nativeptr->begin( );
-
-_GF_MG_EXCEPTION_CATCH_ALL2
+}
+catch (const apache::geode::client::Exception& ex) {
+ throw Apache::Geode::Client::GeodeException::Get(ex);
+}
+catch (System::AccessViolationException^ ex) {
+ throw ex;
+}
}
void CacheTransactionManager::Prepare( )
{
-_GF_MG_EXCEPTION_TRY2
-
+try {
Review comment:
Seems like a nice refactor.
--
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-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Demacroize clicache
>
>
> Key: GEODE-9559
> URL: https://issues.apache.org/jira/browse/GEODE-9559
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Michael Martell
>Priority: Major
> Labels: pull-request-available
>
> Macros in C++ complicate debug efforts and code maintenance and are generally
> considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This
> PR is to remove all the complicated macros in the .NET Framework client, e.g.
> the clicache module.
> In addition to improving the maintainability of the clicache module, removing
> the macros will greatly assist the creation of the .NET Core client. [dotPeek
> |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the
> .NET Core project, but is currently limited by the extensive use of macros in
> the clicache code.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[
https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17408796#comment-17408796
]
ASF GitHub Bot commented on GEODE-9559:
---
mmartell commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r701046962
##
File path: clicache/src/Cache.cpp
##
@@ -159,13 +165,19 @@ namespace Apache
GC::KeepAlive(m_nativeptr);
}
-_GF_MG_EXCEPTION_CATCH_ALL2
+}
+catch (const apache::geode::client::Exception& ex) {
+ throw Apache::Geode::Client::GeodeException::Get(ex);
+}
+catch (System::AccessViolationException^ ex) {
+ throw ex;
+}
}
generic
Client::IRegion^ Cache::GetRegion( String^ path )
{
-_GF_MG_EXCEPTION_TRY2/* due to auto replace */
+try {/* due to auto replace */
Review comment:
Agreed!
--
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-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Demacroize clicache
>
>
> Key: GEODE-9559
> URL: https://issues.apache.org/jira/browse/GEODE-9559
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Michael Martell
>Priority: Major
> Labels: pull-request-available
>
> Macros in C++ complicate debug efforts and code maintenance and are generally
> considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This
> PR is to remove all the complicated macros in the .NET Framework client, e.g.
> the clicache module.
> In addition to improving the maintainability of the clicache module, removing
> the macros will greatly assist the creation of the .NET Core client. [dotPeek
> |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the
> .NET Core project, but is currently limited by the extensive use of macros in
> the clicache code.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[
https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17408794#comment-17408794
]
ASF GitHub Bot commented on GEODE-9559:
---
mmartell commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r701045992
##
File path: clicache/src/Cache.cpp
##
@@ -215,12 +233,18 @@ namespace Apache
GC::KeepAlive(m_nativeptr);
}
-_GF_MG_EXCEPTION_CATCH_ALL2
+}
+catch (const apache::geode::client::Exception& ex) {
+ throw Apache::Geode::Client::GeodeException::Get(ex);
+}
+catch (System::AccessViolationException^ ex) {
Review comment:
Interesting question that deserves some thought. AccessViolations are
bd. So you don't want to just eat them. OK, good, we don't. We rethrow
them. But why catch them in the first place?
The only thing I can come up with is to help track down bugs in our code. We
catch/rethrow AccessViolations all over the place (212 places), so it allows
setting breakpoints which can be caught in the debugger so we can look around.
This may have been useful early on, but I doubt it provides any value now.
I'm good with removing: catch (System::AccessVilationException) everywhere.
--
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-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Demacroize clicache
>
>
> Key: GEODE-9559
> URL: https://issues.apache.org/jira/browse/GEODE-9559
> Project: Geode
> Issue Type: Improvement
> Components: native client
>Reporter: Michael Martell
>Priority: Major
> Labels: pull-request-available
>
> Macros in C++ complicate debug efforts and code maintenance and are generally
> considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This
> PR is to remove all the complicated macros in the .NET Framework client, e.g.
> the clicache module.
> In addition to improving the maintainability of the clicache module, removing
> the macros will greatly assist the creation of the .NET Core client. [dotPeek
> |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the
> .NET Core project, but is currently limited by the extensive use of macros in
> the clicache code.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[ https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17408744#comment-17408744 ] ASF GitHub Bot commented on GEODE-9559: --- pivotal-jbarrett commented on pull request #862: URL: https://github.com/apache/geode-native/pull/862#issuecomment-911555354 > The procedure for this PR consisted of cutting/pasting the original macro invocations with the corresponding preprocessor output. Although there are quite a few files involved, the changes are basically all the same. For this reason it seemed prudent to submit this as a single PR. Is there a way to get the preprocessor to do this for you for specific macros only and then preserve that output so we can be certain there are not copy/paste errors? -- 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-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Demacroize clicache > > > Key: GEODE-9559 > URL: https://issues.apache.org/jira/browse/GEODE-9559 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Martell >Priority: Major > Labels: pull-request-available > > Macros in C++ complicate debug efforts and code maintenance and are generally > considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This > PR is to remove all the complicated macros in the .NET Framework client, e.g. > the clicache module. > In addition to improving the maintainability of the clicache module, removing > the macros will greatly assist the creation of the .NET Core client. [dotPeek > |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the > .NET Core project, but is currently limited by the extensive use of macros in > the clicache code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[
https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17408742#comment-17408742
]
ASF GitHub Bot commented on GEODE-9559:
---
pivotal-jbarrett commented on a change in pull request #862:
URL: https://github.com/apache/geode-native/pull/862#discussion_r700976338
##
File path: clicache/src/Cache.cpp
##
@@ -159,13 +165,19 @@ namespace Apache
GC::KeepAlive(m_nativeptr);
}
-_GF_MG_EXCEPTION_CATCH_ALL2
+}
+catch (const apache::geode::client::Exception& ex) {
+ throw Apache::Geode::Client::GeodeException::Get(ex);
+}
+catch (System::AccessViolationException^ ex) {
+ throw ex;
+}
}
generic
Client::IRegion^ Cache::GetRegion( String^ path )
{
-_GF_MG_EXCEPTION_TRY2/* due to auto replace */
+try {/* due to auto replace */
Review comment:
I think this comment is common and not particularly helpful. Can you
globally delete?
##
File path: clicache/src/CacheableHashSet.hpp
##
@@ -598,69 +604,11 @@ namespace Apache
};
}
-#define _GFCLI_CACHEABLEHASHSET_DEF_GENERIC(m, HSTYPE)
\
- public ref class m : public
Internal::CacheableHashSetType(native::internal::DSCode::m),
HSTYPE> \
-{
\
- public: \
-/**
- * Allocates a new empty instance.
- *
- */ \
- inline m()\
- :
Internal::CacheableHashSetType(native::internal::DSCode::m),
HSTYPE>() {} \
- \
- /**
- * Allocates a new instance with the given size.
- *
- * the initial size of the new instance
- */ \
- inline m(System::Int32 size)
\
- :
Internal::CacheableHashSetType(native::internal::DSCode::m),
HSTYPE>(size) {} \
- \
- /**
-* Static function to create a new empty instance.
-*
-*/ \
-inline static m^ Create() \
- { \
- return gcnew m(); \
- } \
- \
- /**
- * Static function to create a new instance with the given size.
- *
- */ \
- inline static m^ Create(System::Int32 size)
\
- { \
- return gcnew m(size); \
- } \
- \
- /*
- * Factory function to register this class.
- *
- */ \
- static ISerializable^ CreateDeserializable()\
- { \
- return gcnew m(); \
- } \
- \
-internal:
\
- static ISerializable^
Create(std::shared_ptr obj)\
- { \
- return gcnew m(obj);\
- } \
- \
-private:
\
- inline m(std::shared_ptr
nativeptr)\
- :
Internal::CacheableHashSetType(native::internal::DSCode::m),
HSTYPE>(nativeptr) { } \
- };
-
///
/// A mutable ICacheableKey hash set wrapper that can serve as
/// a distributable object for caching.
///
- _GFCLI_CACHEABLEHASHSET_DEF_GENERIC(CacheableHashSet,
-
apache::geode::client::CacheableHashSet);
+ public ref class CacheableHashSet : public
Internal::CacheableHashSetType(native::internal::DSCode::CacheableHashSet),
apache::geode::client::
[jira] [Commented] (GEODE-9559) Demacroize clicache
[ https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17407404#comment-17407404 ] ASF GitHub Bot commented on GEODE-9559: --- mmartell commented on pull request #862: URL: https://github.com/apache/geode-native/pull/862#issuecomment-908649949 The procedure for this PR consisted of cutting/pasting the original macro invocations with the corresponding preprocessor output. Although there are quite a few files involved, the changes are basically all the same. For this reason it seemed prudent to submit this as a single PR. -- 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-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Demacroize clicache > > > Key: GEODE-9559 > URL: https://issues.apache.org/jira/browse/GEODE-9559 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Martell >Priority: Major > Labels: pull-request-available > > Macros in C++ complicate debug efforts and code maintenance and are generally > considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This > PR is to remove all the complicated macros in the .NET Framework client, e.g. > the clicache module. > In addition to improving the maintainability of the clicache module, removing > the macros will greatly assist the creation of the .NET Core client. [dotPeek > |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the > .NET Core project, but is currently limited by the extensive use of macros in > the clicache code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[ https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17406924#comment-17406924 ] ASF GitHub Bot commented on GEODE-9559: --- mmartell commented on pull request #862: URL: https://github.com/apache/geode-native/pull/862#issuecomment-908649949 The procedure for this PR consisted of cutting/pasting the original macro invocations with the corresponding preprocessor output. Although there are quite a few files involved, the changes are basically all the same. For this reason it seemed prudent to submit this as a single PR. -- 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-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Demacroize clicache > > > Key: GEODE-9559 > URL: https://issues.apache.org/jira/browse/GEODE-9559 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Martell >Priority: Major > Labels: pull-request-available > > Macros in C++ complicate debug efforts and code maintenance and are generally > considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This > PR is to remove all the complicated macros in the .NET Framework client, e.g. > the clicache module. > In addition to improving the maintainability of the clicache module, removing > the macros will greatly assist the creation of the .NET Core client. [dotPeek > |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the > .NET Core project, but is currently limited by the extensive use of macros in > the clicache code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[ https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17406320#comment-17406320 ] ASF GitHub Bot commented on GEODE-9559: --- mmartell opened a new pull request #862: URL: https://github.com/apache/geode-native/pull/862 Macros in C++ complicate debug efforts and code maintenance and are [generally considered old school](https://stroustrup.com/icsm-2012-demacro.pdf). This PR is to remove all the complicated macros in the .NET Framework client, e.g. the clicache module. In addition to improving the maintainability of the clicache module, removing the macros will greatly assist the creation of the .NET Core client. [dotPeek ](https://www.jetbrains.com/decompiler/) is proving to be a valuable tool in the .NET Core project, but is currently limited by the extensive use of macros in the clicache 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-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Demacroize clicache > > > Key: GEODE-9559 > URL: https://issues.apache.org/jira/browse/GEODE-9559 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Martell >Priority: Major > > Macros in C++ complicate debug efforts and code maintenance and are generally > considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This > PR is to remove all the complicated macros in the .NET Framework client, e.g. > the clicache module. > In addition to improving the maintainability of the clicache module, removing > the macros will greatly assist the creation of the .NET Core client. [dotPeek > |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the > .NET Core project, but is currently limited by the extensive use of macros in > the clicache code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[ https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17406312#comment-17406312 ] ASF subversion and git services commented on GEODE-9559: Commit 6f61439c258d8458fca331e439e46b3b29fde5ce in geode-native's branch refs/heads/GEODE-9559-demacroize-clicache from Mike Martell [ https://gitbox.apache.org/repos/asf?p=geode-native.git;h=6f61439 ] GEODE-9559: Demacroize _GFCLI_CACHEABLE_KEY_DEF_NEW > Demacroize clicache > > > Key: GEODE-9559 > URL: https://issues.apache.org/jira/browse/GEODE-9559 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Martell >Priority: Major > > Macros in C++ complicate debug efforts and code maintenance and are generally > considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This > PR is to remove all the complicated macros in the .NET Framework client, e.g. > the clicache module. > In addition to improving the maintainability of the clicache module, removing > the macros will greatly assist the creation of the .NET Core client. [dotPeek > |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the > .NET Core project, but is currently limited by the extensive use of macros in > the clicache code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[ https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17406313#comment-17406313 ] ASF subversion and git services commented on GEODE-9559: Commit c8af94e12a4b420e079cf8a2cb6a13ce0e1298cc in geode-native's branch refs/heads/GEODE-9559-demacroize-clicache from Mike Martell [ https://gitbox.apache.org/repos/asf?p=geode-native.git;h=c8af94e ] GEODE-9559: Demacroize _GFCLI_CACHEABLEHASHSET_DEF_GENERIC > Demacroize clicache > > > Key: GEODE-9559 > URL: https://issues.apache.org/jira/browse/GEODE-9559 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Martell >Priority: Major > > Macros in C++ complicate debug efforts and code maintenance and are generally > considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This > PR is to remove all the complicated macros in the .NET Framework client, e.g. > the clicache module. > In addition to improving the maintainability of the clicache module, removing > the macros will greatly assist the creation of the .NET Core client. [dotPeek > |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the > .NET Core project, but is currently limited by the extensive use of macros in > the clicache code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (GEODE-9559) Demacroize clicache
[ https://issues.apache.org/jira/browse/GEODE-9559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17406314#comment-17406314 ] ASF subversion and git services commented on GEODE-9559: Commit afe9584917027e2c9e94db3fa8eb25e092a8cc3c in geode-native's branch refs/heads/GEODE-9559-demacroize-clicache from Mike Martell [ https://gitbox.apache.org/repos/asf?p=geode-native.git;h=afe9584 ] GEODE-9559: Forgot to rm _GFCLI_CACHEABLEHASHSET_DEF_GENERIC > Demacroize clicache > > > Key: GEODE-9559 > URL: https://issues.apache.org/jira/browse/GEODE-9559 > Project: Geode > Issue Type: Improvement > Components: native client >Reporter: Michael Martell >Priority: Major > > Macros in C++ complicate debug efforts and code maintenance and are generally > considered old school ([https://stroustrup.com/icsm-2012-demacro.pdf).] This > PR is to remove all the complicated macros in the .NET Framework client, e.g. > the clicache module. > In addition to improving the maintainability of the clicache module, removing > the macros will greatly assist the creation of the .NET Core client. [dotPeek > |http://jetbrains.com/decompiler/] is proving to be a valuable tool in the > .NET Core project, but is currently limited by the extensive use of macros in > the clicache code. -- This message was sent by Atlassian Jira (v8.3.4#803005)
