Просмотр исходного кода

Add an invalid #define for SDL_ThreadID() to SDL_oldnames.h (#15801)

in SDL2 SDL_ThreadID() was a function that's now
SDL_GetCurrentThreadID(), but in SDL3 SDL_ThreadID is a type, so
in C++ `x = SDL_ThreadID()` is valid code (default constructor which
in case of integers means 0), so that's a massive footgun.

See the big comment in SDL_oldnames.h for more details.

Added `#undef SDL_ThreadID` in SDL_dynapi.c because it has one of the
(quite rare) cases  where "SDL_ThreadID" followed by a "(" is actually
correct and necessary (function pointer returning SDL_ThreadID).
Daniel Gibson 13 часов назад
Родитель
Сommit
b04d026458
3 измененных файлов с 71 добавлено и 0 удалено
  1. 11 0
      docs/README-migration.md
  2. 54 0
      include/SDL3/SDL_oldnames.h
  3. 6 0
      src/dynapi/SDL_dynapi.c

+ 11 - 0
docs/README-migration.md

@@ -41,6 +41,8 @@ Many functions and symbols have been renamed. We have provided a handy Python sc
 rename_symbols.py --all-symbols source_code_path
 ```
 
+**Note** that `rename_symbols.py` can't completely handle `SDL_ThreadID`, see the [SDL_Thread.h section](#sdl_threadh) below.
+
 It's also possible to apply a semantic patch to migrate more easily to SDL3: [SDL_migration.cocci](https://github.com/libsdl-org/SDL/blob/main/build-scripts/SDL_migration.cocci)
 
 SDL headers should now be included as `#include <SDL3/SDL.h>`. Typically that's the only SDL header you'll need in your application unless you are using OpenGL or Vulkan functionality. SDL_image, SDL_mixer, SDL_net, SDL_ttf and SDL_rtf have also their preferred include path changed: for SDL_image, it becomes `#include <SDL3_image/SDL_image.h>`. We have provided a handy Python script [rename_headers.py](https://github.com/libsdl-org/SDL/blob/main/build-scripts/rename_headers.py) to rename SDL2 headers to their SDL3 counterparts:
@@ -2046,6 +2048,15 @@ The following functions have been removed:
 The following symbols have been renamed:
 * SDL_threadID => SDL_ThreadID
 
+### Attention: Potential problems with `SDL_ThreadID`
+
+**_`SDL_ThreadID`_** is a case that needs **special attention**, because if you forget to rename a call to `SDL_ThreadID()` to `SDL_GetCurrentThreadID()` in **C++** code, it will *still compile fine*, calling the default constructor of the `SDL_ThreadID` type, initializing it to `0` - but if `SDL_GetCurrentThreadID()` was intended, that of course leads to **wrong behavior** that can be hard to catch.
+
+Furthermore **`rename_symbols.py` does not handle it** (because it would be hard to do in a way that doesn't break anything when running that script twice), which makes it even more error-prone.
+
+SDL_oldnames.h now has a `#define` that should catch these cases, but it's not available in SDL 3.4.10 and older. So if you're using 3.4.10 or older, check your code carefully for occurences of `SDL_ThreadID()` and make sure it wasn't supposed to be `SDL_GetCurrentThreadID()`.  
+In some edge cases this `#define` can cause false positives (compile errors even though `SDL_ThreadID` was used correctly); right above that definition in the header is a long comment explaining possible problems and solutions.
+
 ## SDL_timer.h
 
 SDL_GetTicks() now returns a 64-bit value. Instead of using the SDL_TICKS_PASSED macro, you can directly compare tick values, e.g.

+ 54 - 0
include/SDL3/SDL_oldnames.h

@@ -1336,4 +1336,58 @@
 
 #endif /* SDL_ENABLE_OLD_NAMES */
 
+/* In SDL2, `SDL_ThreadID()` was a function that is now called `SDL_GetCurrentThreadID()`.
+ * In SDL3, the thread ID *type* is called `SDL_ThreadID` (in SDL2 it was
+ * `SDL_threadID` with lower 't').
+ *
+ * Unfortunately, at least in C++ writing SDL_ThreadID() compiles fine even for
+ * the type, it's the default constructor of the type, so e.g.
+ * `myID = SDL_ThreadID();` is equivalent to `myID = 0;`
+ *
+ * Of course if you've been porting SDL2 code and have missed renaming this case
+ * of `SDL_ThreadID()` to `SDL_GetCurrentThreadID()` this is quite a pitfall:
+ * The code compiles fine, but behaves wrong: it will set `myID` to `0` instead
+ * of the current thread's ID.
+ *
+ * This makes it impossible to provide a "proper" `SDL_ENABLE_OLD_NAMES` treatment
+ * of SDL_ThreadID() in this header, so the following `#define` sets `SDL_ThreadID()`
+ * to an invalid value, forcing you to consciously adjust it to your needs, i.e.
+ * replacing it with `SDL_GetCurrentThreadID()` or `0` depending on your intentions.
+ *
+ * Note that using `SDL_ThreadID x = ...;` is unaffected, only "SDL_ThreadID" followed
+ * by "(" will lead to a compile error.
+ *
+ * Unfortunately, there are some cases where this breaks legit SDL3 code that
+ * actually means to use `SDL_ThreadID(...`, like:
+ * - Definitions of function pointers with SDL_ThreadID as return type, like
+ *   `typedef SDL_ThreadID (SDLCALL *SDL_DYNAPIFN_SDL_GetCurrentThreadID) (void);`
+ * - C++ casts like `SDL_ThreadID tid = SDL_ThreadID(id);`
+ *
+ * For C++ casts, just replace `SDL_ThreadID(id)` with either a C-style cast
+ * ( `(SDL_ThreadID)id` ) or one of the other C++ cast types like
+ * `static_cast<SDL_ThreadID>(id)`.
+ *
+ * For the (hopefully rare) case of function pointers returning SDL_ThreadID,
+ * or any other legit case where `SDL_ThreadID` is followed by `(` that can't be
+ * easily avoided by using slightly different syntax, you can `#undef SDL_ThreadID`
+ * before your code using it to get rid of this define causing the compiler error.
+ *
+ * If you're using `SDL_ENABLE_OLD_NAMES` to support both SDL2 and SDL3 with
+ * the same code, consider adding:
+ *
+ * ```c
+ * #if SDL_MAJOR_VERSION == 2
+ *   #define SDL_GetCurrentThreadID() SDL_ThreadID()
+ * #endif
+ * ```
+ *
+ * and using `SDL_GetCurrentThreadID()` in your code, even if you otherwise use
+ * the SDL2 names.
+ *
+ * The gain of catching the bugs caused by accidentally using `SDL_ThreadID()`
+ * when `SDL_GetCurrentThreadID()` was intended hopefully outweight the annoyances
+ * caused by this in some rare cases.
+ */
+#define SDL_ThreadID()  SDL_ThreadID_renamed_SDL_GetCurrentThreadID
+
 #endif /* SDL_oldnames_h_ */

+ 6 - 0
src/dynapi/SDL_dynapi.c

@@ -55,6 +55,12 @@
 #include <windows.h>
 #endif
 
+#ifdef SDL_ThreadID
+/* prevent the SDL_ThreadID() define from SDL_oldnames.h from breaking
+   function pointer definitions */
+#undef SDL_ThreadID
+#endif
+
 /* This is the version of the dynamic API. This doesn't match the SDL version
    and should not change until there's been a major revamp in API/ABI.
    So 2.0.5 adds functions over 2.0.4? This number doesn't change;