SFML community forums

General => SFML development => Topic started by: Nexus on April 07, 2015, 11:34:22 pm

Title: API deprecation model
Post by: Nexus on April 07, 2015, 11:34:22 pm
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:
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.
Title: Re: API deprecation model
Post by: Juhani on April 08, 2015, 01:38:48 am
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.
Title: Re: API deprecation model
Post by: eXpl0it3r on April 08, 2015, 02:12:13 am
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.
Title: Re: API deprecation model
Post by: Lo-X on April 08, 2015, 03:44:26 am
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.

Title: Re: API deprecation model
Post by: binary1248 on April 08, 2015, 03:49:27 am
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:
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.
Title: Re: API deprecation model
Post by: Hiura on April 08, 2015, 10:50:45 am
  • 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.
Title: Re: API deprecation model
Post by: minirop on April 08, 2015, 12:58:59 pm
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.
Title: Re: API deprecation model
Post by: Mario on April 08, 2015, 08:39:20 pm
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).
Title: Re: API deprecation model
Post by: Nexus on April 08, 2015, 09:40:42 pm
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!
Title: Re: API deprecation model
Post by: Mario on April 09, 2015, 08:47:57 am
Didn't check the other compilers so far, but at least for MSVC #pragma deprecated will work with any identifier.
Title: Re: API deprecation model
Post by: Hiura on April 09, 2015, 09:25:47 am
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.
Title: Re: API deprecation model
Post by: Foaly on September 18, 2015, 02:01:22 pm
I needed this functionality for the multi monitor support (http://en.sfml-dev.org/forums/index.php?topic=18950.0) 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) (https://github.com/Foaly/SFML/blob/ee27023318d223de9dc91ef4e149aa31d0a19f5f/include/SFML/Config.hpp#L160)

It is basically a simplified version of the openFrameworks deprecation macro (https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/utils/ofConstants.h#L35). 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 (https://gcc.gnu.org/onlinedocs/gcc/Enumerator-Attributes.html) it should also work with enums but I haven't tested it.
Title: Re: API deprecation model
Post by: GraphicsWhale on September 18, 2015, 03:00:18 pm
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?  ;D

http://docs.oracle.com/javase/1.5.0/docs/api/java/awt/FontMetrics.html#getMaxDecent%28%29 (http://docs.oracle.com/javase/1.5.0/docs/api/java/awt/FontMetrics.html#getMaxDecent%28%29)
Title: Re: API deprecation model
Post by: Mario on September 18, 2015, 08:11:00 pm
        #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).
Title: Re: API deprecation model
Post by: Jesper Juhl on September 18, 2015, 08:18:33 pm
Once SFML gets to enjoy the goodness that is C++14 there's also [deprecated] (http://en.cppreference.com/w/cpp/language/attributes).
Title: Re: API deprecation model
Post by: Foaly on September 18, 2015, 08:21:57 pm
[deprecated] is only available in C++14 as far as I know.

@Mario: Thanks! Good catch. I'll report that to the openFrameworks developers!

edit: whoops I was to quick ;) This article (http://josephmansfield.uk/articles/marking-deprecated-c++14.html) seems to explain it quiet well. According to the article it is already built in Clang 3.4 and GCC 4.9
Title: Re: API deprecation model
Post by: Klaim on September 19, 2015, 03:18:10 am
[[deprecated]] and other non-standard C++ compiler-specific intrinsics can be used as an implementation detail of a deprecated-function-marking macro. IMMIC they are all compatible in that you can use them in the same places in code.

Just some input from how we do it in my dayjob (humanoid robot APIs, which have the same kind of issues because interface evolve a lot but most interfaces need to be usable for a long time):

 1. Compile Time: we have a macro using whatever the compiler have available to trigger warning to users of functions and types we mark with that macro. It indeed works well at compile time IF you manage to not have any other warnings on all your supported plateform. We use Boost so we get often tons of warning on some plateforms so it works but we have to patch Boost each time we upgrade it. I don't think SFML would have this kind of problem.

 2. Runtime: we have a logging system and we add a log at "warning" level each in the implementation of each function we mark as deprecated. It helps in our case because we have some magic making our API available through RPC calls automatically, so most APIs are actually messages between processes and when APIs evolve, your client code should still work after at least one iteration of the API. Which basically mean that users might not compile your API code and just do RPC calls so the only way for them to know (in this case) that they are calling a deprecated API is at runtime.
   The obvious side effect is that logging happen even in previously very fast API functions, though it's not yet an issue for us, at least not for deprecated APIs as we are not yet reaching the point where we will need to have tighter performance (it's not one application, its tons of them, so it's almost impossible to try to improve performance on a case by case, we mostly do performance optimization in inter-process and multithreading systems).
   The other issue is that it relies on the fact that the user will use our logging system, which he will in our very specific case but can't work with SFML I believe. I don't know what would work in the case of SFML for this.
Title: Re: API deprecation model
Post by: Nexus on September 19, 2015, 03:32:22 pm
I created a pull request here: https://github.com/SFML/SFML/pull/969

Concerning runtime warnings/logs: that's definitely something to consider. I think it should be part of our overall log/error output redesign.
Title: Re: API deprecation model
Post by: Klaim on September 19, 2015, 04:35:13 pm
Concerning runtime warnings/logs: that's definitely something to consider. I think it should be part of our overall log/error output redesign.

Cool! Is there a specific thread where you are discussing/anouncing this? I searched with these words but didn't find anything recent. I don't follow much these forums but I'm interested in the rationals for these kinds of "looks simple but actually hard" topics.