Remove old #if hacks for older compilers from Delegate
#1017
Conversation
That bug was closed 15 years ago. This project targets C++17. There's no way that's still relevant.
Function type syntax is part of C++17, so will be supported by any targetted compilers.
Version 1300 corresponds with Visual Studio 2002 7.0. That definitely won't have C++17 support.
When compiling with MSVC, this feature will be present on all versions supporting C++17 or newer.
| @@ -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; | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Reference: #1015
The code requires at least C++17 support, so any special hacks for compilers too old to support C++17 aren't needed.
Might be good if a Windows developer tested OPHD still compiles and works with this change, since the removed hacks are compiler and platform specific.