API deprecation model

[Nexus]

With sf::Event::MouseWheelMoved, SFML starts to mark parts of its API as deprecated. Deprecated features will continue to work in existing code, but their use is discouraged in new code. These features will be removed in the next major version, that is, SFML 3.

The library should encourage a fast transition from deprecated features to their superior counterparts. At the moment, it does so using annotations in the API documentation. However, this is not enough, as people used to a feature will continue to use it, without consulting the documentation again. Expecting people to constantly look up the documentation for working code is naive; nobody does that. At best, a small fraction of people has a look at Doxygen's deprecation list from time to time.

That's why I think it would be nice to have a stronger mechanism to make people aware when they use deprecated features. I have several ideas in mind, for each build step:

• Compile time: There are compiler-specific keywords (__declspec(deprecated) and #pragma deprecated in MSVC, __attribute__((deprecated)) in g++), however their use is limited to some C++ identifiers (mainly functions). While this is not relevant at the time being, C++14 introduces the [[deprecated]] attribute as a general-purpose and portable feature.
  
• Preprocessor time: A possible addition would be a macro SFML_NO_DEPRECATED for conditional compilation, in which all the deprecated features are disabled. This sounds very good, but there is one problem: conditionally compiled variants of the library lead to different binaries -- already because the symbol in question is no longer exported in dynamic libraries. If we consider how many people even get the simple SFML_STATIC wrong, I don't want to imagine how many people encounter subtle linker or even runtime errors in this case.
  
• Runtime: The sf::err() stream could be used to emit warnings when deprecated features are used. This requires additional code in corresponding functions as well as a mechanism to enable/disable the warnings. It detects the deprecation only late (possibly never), but is binary compatible.

What do you think? Before you argue from your own standpoint as an occasional SFML user and say "it's exaggerated", consider that the whole point of using deprecation in the first place is to avoid API-breaking changes caused by removal of functionality. SFML has not introduced deprecation primarily for occasional projects which can be adapted in 15 minutes. It's rather something that concerns long-term projects, potentially commercial -- these projects are the ones that benefit most from early recognition of deprecated features.

[Juhani]

I think a serious commercial project should at least check the changelog. If it starts with something like

!!!ATTENTION!!! The following features have been **deprecated** in version x.y:

they should notice.

[eXpl0it3r]

Personally I think if we have some warn system like that it needs to be opt-in, but once it's opt-in, you'll already be aware that there are some deprecated features which you already might have looked up in the docs.
And given that we're not trying to deprecate all kinds of stuff, it shouldn't really be too hard to figure them out.

But then again it could be useful for some bigger projects to add it to their standard build config and get notified if someone uses something deprecated.

Number 1 can't be done (easily) right now.
For the other ones I'm not really sure which one would work better.

[Lo-X]

I think you should put warning at compilation if possible and why not at execution. Warnings shouldn't be easily avoidable, if they can be (meaning: you have them by default and have to do something specific to ignore them).

But the most important point is: what to use instead of deprecated methods and how ? This should be easy to figure out from the warnings and should require the most minimal documentation research.

[binary1248]

I think providing a bit more information than a simple note in the documentation is a good idea.

I don't know if you were implying this or not, but I think the 3 points you listed shouldn't be considered separately. There are really 3 scenarios to think of:

• The developer doesn't know about deprecation (yet).
• The developer knows about deprecation but simply doesn't care since they intend to not use future versions of the library in their current project.
• The developer knows about deprecation and does care about making sure their code stays future-proof in case they might consider upgrading to SFML 3 in the future.

We can differentiate between the first and latter 2 cases through a preprocessor define. By default, deprecation warnings (in all forms) will be enabled, so unless the developer is ignorant and completely oblivious to their own build/testing process, they really cannot oversee this.

Once they get the message and might be annoyed by it, they try to figure out how to get rid of all the warnings. That's where the preprocessor define comes in. Either they care about deprecation or they don't, so they can choose to define SFML_ALLOW_DEPRECATED or not. In the former case, it would lead to all deprecation warnings being disabled, in the latter case, the developer would actively try to address each warning one by one. Either way, once they are done the warnings will disappear.

I don't think we have to go ahead and remove symbols from the library based on deprecation just yet. It is deprecation after all. Linker errors would not be a problem in this case. We only start removing stuff in SFML 3, once developers have had enough warning and time to prepare. If their application fails to link then because of removed features, then it really isn't our fault, we gave them enough warnings.

How we choose to emit warnings has to be considered on a toolchain basis. Like you said, many of the possibilities aren't standardized (or widely available yet), so I think just giving our best effort for the biggest toolchains should satisfy almost all SFML users.

If we did go down the route of emitting warnings to the console during runtime as well, we should really start to consider implementing separate streams like I already mentioned in the past. Dumping everything to sf::err() just seems sloppy if you ask me. We should have in addition to sf::err() something like sf::warn(), sf::info() and sf::debug() as well, which can be disabled in release builds if so desired.

[Hiura]

• Compile time:[...]
  
• Preprocessor time:[...]
  
• Runtime:[...]
  

FYI, Apple does both 1 and 3 (not in stdout but in the system log).

What about other big projects? (Qt, boost, ...) Has someone any experience with those?

Dumping everything to sf::err() just seems sloppy if you ask me. We should have in addition to sf::err() something like sf::warn(), sf::info() and sf::debug() as well, which can be disabled in release builds if so desired.

I think we need to seriously consider looking into a proper logging system, indeed.

[minirop]

What about other big projects? (Qt, boost, ...) Has someone any experience with those?

I just watched the source code of Qt, and they use 1. and 2. for methods, and simple a note for things like enum values (they use 3. only(?) for invalid arguments).

#ifdef QT_DEPRECATED
#define QT_DEPRECATED_SINCE(major, minor) (QT_VERSION_CHECK(major, minor, 0) > QT_DISABLE_DEPRECATED_BEFORE)
#else
#define QT_DEPRECATED_SINCE(major, minor) 0
#endif
// ...
#if QT_DEPRECATED_SINCE(5, 0)
    QT_DEPRECATED  char toAscii() const { return QChar(*this).toLatin1(); }
#endif
// QT_DEPRECATED expands to __attribute__ ((__deprecated__)) or __declspec(deprecated) or etc.

[Mario]

I'd vote for a combination of 1+3. Excluding deprecated stuff right now sounds like there's no gain other than you being able to "test" code to see whether it will still compile (but you've got the warnings for that anyway).

[Nexus]

As I mentioned, the main problem of the compiler warnings is that they cannot be applied to all C++ symbols. Especially in the current case, where the deprecated symbol is an enumerator, we cannot do anything at compile time. Unfortunately, even C++14 [[deprecated]] does not cover that case for some odd reason.

About the logging system: that's true, but even then we need a separate switch to turn such warnings off, possibly in the form of a global function. And we need to discuss it before a new logging system... If we want to make people aware of deprecated feature, there's no reason not to start already at SFML 2.3 (especially for minor changes).

Qt seems to have quite a sophisticated mechanism, thanks for looking it up!

[Mario]

Didn't check the other compilers so far, but at least for MSVC #pragma deprecated will work with any identifier.

[Hiura]

It's actually easy with clang ( http://clang.llvm.org/docs/LanguageExtensions.html#attributes-on-enumerators ), through I haven't checked the availability of this through the different version of clang.

I couldn't find out if gcc supported the same syntax or not. The doc says nothing about enums(?)

Regarding msvc, __declspec might do the trick but again I didn't find anything enum related. ( https://msdn.microsoft.com/en-us/library/dabb5z75.aspx )

And finally, we would need a way to make this work (through a macro) for all of them with only one syntax.

Alternatively, or additionally, we might want deprecate the variable inside the union corresponding to this enum value. Note that this will trigger some warnings during SFML compilation but that should be ok.

[Foaly]

I needed this functionality for the multi monitor support and I didn't know this had been discussed or that this thread existed.
So i just went ahead and implemented it myself. Here is what the code looks like:

////////////////////////////////////////////////////////////
// Cross-platform deprecation warning
////////////////////////////////////////////////////////////
#ifdef __GNUC__
        // clang also has this defined. deprecated(message) is only for gcc >= 4.5
        #if (__GNUC__ >= 4) && (__GNUC_MINOR__ >= 5)
        #define SFML_DEPRECATED(message, func) func __attribute__ ((deprecated(message)))
    #else
        #define SFML_DEPRECATED(message, func) func __attribute__ ((deprecated))
    #endif
#elif defined(_MSC_VER)
        #define SFML_DEPRECATED(message, func) __declspec(deprecated(message)) func
#else
        #warning WARNING: This compiler does not support SFML_DEPRECATED
        #define SFML_DEPRECATED(message, func) func
#endif

(Github for better highlighting)

It is basically a simplified version of the openFrameworks deprecation macro. I have been using it in the past and it worked very good. What I liked especially about it is that it not only emits a warning, but also prints a message. Of course the message has to be set to something helpful that tells the user what to use instead. Example what it could look like:
/home/WAGE06/playground/Foaly_SFML/Source/src/SFML/Window/Unix/WindowImplX11.cpp|1134|warning: ‘static sf::VideoMode sf::VideoMode::getDesktopMode()’ is deprecated (declared at /home/playground/Foaly_SFML/Source/include/SFML/Window/VideoMode.hpp:70): sf::VideoMode::getDesktopMode() is deprected, use sf::Screen::get(0).getDesktopMode() instead. [-Wdeprecated-declarations]

I have used it with variables, types and functions on gcc and experienced no problems. According to this it should also work with enums but I haven't tested it.

[GraphicsWhale]

I think the first is best. A simple compiler warning is enough.

With sf::Event::MouseWheelMoved, SFML starts to mark parts of its API as deprecated.

From the documentation:
This event is deprecated and potentially inaccurate. Use MouseWheelScrollEvent instead.

Did you guys by any chance also write the Java standard library? 

http://docs.oracle.com/javase/1.5.0/docs/api/java/awt/FontMetrics.html#getMaxDecent%28%29

[Mario]

        #if (__GNUC__ >= 4) && (__GNUC_MINOR__ >= 5)
        #define SFML_DEPRECATED(message, func) func __attribute__ ((deprecated(message)))

This condition check has to be changed. Otherwise this won't apply to versions like 5.0 (since minor is no longer bigger than or equal to 5).

[Jesper Juhl]

Once SFML gets to enjoy the goodness that is C++14 there's also [deprecated].