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

Remove old #if hacks for older compilers from Delegate #1017

Merged
merged 7 commits into from
Dec 13, 2021
Merged

Remove old #if hacks for older compilers from Delegate #1017

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
462da28
Remove workaround for old GCC compilers
DanRStevens Jul 15, 2021
ad315cc
Remove function type syntax guard
DanRStevens Jul 15, 2021
f958f6c
Remove no longer referenced macro define
DanRStevens Jul 15, 2021
1aa3e53
Adjust namespace blank line padding
DanRStevens Jul 15, 2021
ef703d5
Remove conditionals for always defined macro
DanRStevens Jul 15, 2021
96e7900
Remove check for ancient version of MSVC
DanRStevens Jul 15, 2021
2f02cae
Remove nested define for `__single_inheritance` feature check
DanRStevens Dec 8, 2021
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
120 changes: 0 additions & 120 deletions NAS2D/Signal/Delegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,36 +28,17 @@
#include <cstring>


#define FASTDELEGATE_USESTATICFUNCTIONHACK

// Compiler identification. It's not easy to identify Visual C++ because many vendors
// fraudulently define Microsoft's identifiers.
#if defined(_MSC_VER) && !defined(__MWERKS__) && !defined(__VECTOR_C) && !defined(__ICL) && !defined(__BORLANDC__)
#define FASTDLGT_ISMSVC
#define FASTDLGT_MICROSOFT_MFP
#define FASTDLGT_HASINHERITANCE_KEYWORDS
#endif

// Does it allow function declarator syntax? The following compilers are known to work:
#if defined(FASTDLGT_ISMSVC) && (_MSC_VER >= 1310) // VC 7.1
#define FASTDELEGATE_ALLOW_FUNCTION_TYPE_SYNTAX
#endif

// Gcc(2.95+), and versions of Digital Mars, Intel and Comeau in common use.
#if defined(__DMC__) || defined(__GNUC__) || defined(__ICL) || defined(__COMO__)
#define FASTDELEGATE_ALLOW_FUNCTION_TYPE_SYNTAX
#endif

#ifdef __GNUC__ // Workaround GCC bug #8271
#define FASTDELEGATE_GCC_BUG_8271
#endif

namespace NAS2D
{

namespace detail
{

template <typename OutputClass, typename InputClass>
union horrible_union
{
Expand Down Expand Up @@ -123,10 +104,7 @@ namespace NAS2D
// GenericClass is a fake class, ONLY used to provide a type. It is vitally important
// that it is never defined.
#ifdef FASTDLGT_MICROSOFT_MFP

#ifdef FASTDLGT_HASINHERITANCE_KEYWORDS
class __single_inheritance GenericClass;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also be removed since its outer #ifdef check was also removed. Additionally, the identifier uses syntax that is reserved; in this case, the variable identifier contains double underscores.

Ref: https://en.cppreference.com/w/cpp/language/identifiers#In_declarations

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, that code was normally enabled for MSVC. There are two nested #ifdef checks, both of which were permanently enabled for MSVC. I removed one of them, related to the __single_inheritance feature check, since that feature was added long ago, so any version supporting C++17 will have it available. The only check that's really relevant is if the code is being compiled by MSVC or not.

Btw, __single_inheritance is an MSVC specific keyword here, not an identifier. It affects the size of pointer to member functions, when the pointer is taken on an incomplete type, so the compiler is not yet able to deduce the inheritance being used.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I learned something today. That said, I really try to avoid using compiler-specific extensions. It just makes things harder.

The article linked remarks that the pointers_to_members pragma can be used instead and since it is the C++-specific alternative. (i.e., not Microsoft-specific, i.e. cross-platform) I would lean towards using that instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Always good to learn something new. 🙂

The __single_inheritance keyword is class specific: (Emphasis mine)

  • At the command line using the /vmg switch
  • Using the pointers_to_members pragma
  • Using the inheritance keywords __single_inheritance, __multiple_inheritance, and __virtual_inheritance. This technique controls the inheritance model on a per-class basis.

The other methods set a default that applies to all classes. We may want to avoid changing the default and potentially affecting other classes that we didn't mean to impact. The pointers_to_members pragma page comes with a warning:

Caution

We advise you to put the pointers_to_members pragma only in the source code file that you want to affect, and only after any #include directives. This practice reduces the risk that the pragma will affect other files, and that you'll accidentally specify multiple definitions for the same variable, function, or class name.

Keep in mind that Delegate is a template class, so all code is in a header file, which means any including file would be impacted by a pragma that changes a compiler default.

Perhaps more to the point though, that use of __single_inheritance was pre-existing, so somewhat out of scope for this PR. I hadn't looked into its use very carefully, so it wasn't something I really wanted to change.

#endif
class GenericClass {};
#else
class GenericClass;
Expand Down Expand Up @@ -232,9 +210,6 @@ namespace NAS2D
};


#if (_MSC_VER <1300)
#error Support for this compiler is no longer provided. Please upgrade.
#else
template <>
struct SimplifyMemFunc<SINGLE_MEMFUNCPTR_SIZE + 3 * sizeof(int)>
{
Expand Down Expand Up @@ -266,7 +241,6 @@ namespace NAS2D
};
};
#endif
#endif
}


Expand All @@ -277,26 +251,7 @@ namespace NAS2D
detail::GenericClass* m_pthis;
GenericMemFuncType m_pFunction;

#if !defined(FASTDELEGATE_USESTATICFUNCTIONHACK)
using GenericFuncPtr = void (*)();
GenericFuncPtr m_pStaticFunction;
#endif

public:
#if !defined(FASTDELEGATE_USESTATICFUNCTIONHACK)
DelegateMemento() :
m_pthis(nullptr),
m_pFunction(nullptr),
m_pStaticFunction(nullptr),
{}

void clear()
{
m_pthis = nullptr;
m_pFunction = nullptr;
m_pStaticFunction = nullptr;
}
#else
DelegateMemento() :
m_pthis(nullptr),
m_pFunction(nullptr)
Expand All @@ -307,33 +262,13 @@ namespace NAS2D
m_pthis = nullptr;
m_pFunction = nullptr;
}
#endif
public:
#if !defined(FASTDELEGATE_USESTATICFUNCTIONHACK)
inline bool IsEqual(const DelegateMemento& x) const
{
if (m_pFunction != x.m_pFunction) return false;
if (m_pStaticFunction != x.m_pStaticFunction) return false;
if (m_pStaticFunction)
{
return m_pthis == x.m_pthis;
}
else
{
return true;
}
}
#else
inline bool IsEqual(const DelegateMemento& x) const
{
return m_pthis == x.m_pthis && m_pFunction == x.m_pFunction;
}
#endif
inline bool IsLess(const DelegateMemento& right) const
{
#if !defined(FASTDELEGATE_USESTATICFUNCTIONHACK)
if (m_pStaticFunction || right.m_pStaticFunction) return m_pStaticFunction < right.m_pStaticFunction;
#endif
if (m_pthis != right.m_pthis) return m_pthis < right.m_pthis;

return memcmp(&m_pFunction, &right.m_pFunction, sizeof(m_pFunction)) < 0;
Expand All @@ -355,19 +290,13 @@ namespace NAS2D
DelegateMemento(const DelegateMemento& right) :
m_pthis(right.m_pthis),
m_pFunction(right.m_pFunction)
#if !defined(FASTDELEGATE_USESTATICFUNCTIONHACK)
, m_pStaticFunction(right.m_pStaticFunction)
#endif
{}

protected:
void SetMementoFrom(const DelegateMemento& right)
{
m_pFunction = right.m_pFunction;
m_pthis = right.m_pthis;
#if !defined(FASTDELEGATE_USESTATICFUNCTIONHACK)
m_pStaticFunction = right.m_pStaticFunction;
#endif
}
};

Expand All @@ -382,30 +311,13 @@ namespace NAS2D
inline void bindmemfunc(X* pthis, XMemFunc function_to_bind)
{
m_pthis = SimplifyMemFunc<sizeof(function_to_bind)>::Convert(pthis, function_to_bind, m_pFunction);
#if !defined(FASTDELEGATE_USESTATICFUNCTIONHACK)
m_pStaticFunction = nullptr;
#endif
}

template <typename X, typename XMemFunc>
inline void bindconstmemfunc(const X* pthis, XMemFunc function_to_bind)
{
m_pthis = SimplifyMemFunc<sizeof(function_to_bind)>::Convert(const_cast<X*>(pthis), function_to_bind, m_pFunction);
#if !defined(FASTDELEGATE_USESTATICFUNCTIONHACK)
m_pStaticFunction = nullptr;
#endif
}

#ifdef FASTDELEGATE_GCC_BUG_8271 // At present, GCC doesn't recognize constness of MFPs in templates
template <typename X, typename XMemFunc>
inline void bindmemfunc(const X* pthis, XMemFunc function_to_bind)
{
bindconstmemfunc(pthis, function_to_bind);
#if !defined(FASTDELEGATE_USESTATICFUNCTIONHACK)
m_pStaticFunction = nullptr;
#endif
}
#endif

inline GenericClass* GetClosureThis() const
{
Expand All @@ -417,32 +329,6 @@ namespace NAS2D
return CastMemFuncPtr<GenericMemFunc>(m_pFunction);
}

#if !defined(FASTDELEGATE_USESTATICFUNCTIONHACK)

public:
template <typename DerivedClass>
inline void CopyFrom(DerivedClass* pParent, const DelegateMemento& x)
{
SetMementoFrom(x);
if (m_pStaticFunction) m_pthis = reinterpret_cast<GenericClass*>(pParent);
}

template <typename DerivedClass, typename ParentInvokerSig>
inline void bindstaticfunc(DerivedClass* pParent, ParentInvokerSig static_function_invoker, StaticFuncPtr function_to_bind)
{
if (!function_to_bind)
{
m_pFunction = nullptr;
}
else
{
bindmemfunc(pParent, static_function_invoker);
}
m_pStaticFunction = reinterpret_cast<GenericFuncPtr>(function_to_bind);
}
inline UnvoidStaticFuncPtr GetStaticFunction() const { return reinterpret_cast<UnvoidStaticFuncPtr>(m_pStaticFunction); }
#else

template <typename DerivedClass>
inline void CopyFrom(DerivedClass*, const DelegateMemento& right)
{
Expand All @@ -469,7 +355,6 @@ namespace NAS2D
static_assert(sizeof(UnvoidStaticFuncPtr) != sizeof(this), "Can't use evil method");
return horrible_cast<UnvoidStaticFuncPtr>(this);
}
#endif

inline bool IsEqualToStaticFuncPtr(StaticFuncPtr funcptr)
{
Expand All @@ -483,7 +368,6 @@ namespace NAS2D
}
}
};

}


Expand Down Expand Up @@ -588,8 +472,6 @@ namespace NAS2D
};


#ifdef FASTDELEGATE_ALLOW_FUNCTION_TYPE_SYNTAX

template <typename Signature>
class Delegate;

Expand Down Expand Up @@ -623,8 +505,6 @@ namespace NAS2D
}
};

#endif

template <typename X, typename Y, typename RetType, typename... Params>
DelegateX<RetType, Params...> MakeDelegate(Y* x, RetType (X::*func)(Params...))
{
Expand Down