libgd / libgd

GD Graphics Library
http://libgd.org
Other
905 stars 272 forks source link

Deprecate gdImageDashedLine()? #435

Open cmb69 opened 6 years ago

cmb69 commented 6 years ago

The former GD manual notes wrt. gdImageDashedLine():

gdImageDashedLine is provided solely for backwards compatibility with gd 1.0. New programs should draw dashed lines using the normal gdImageLine function and the new gdImageSetStyle function.

However, we do not mention this anymore. This raises the question whether we should stick with gdImageDashedLine() or whether we should actually deprecate this function.

Note that downstream projects may already have heeded the deprecation. For instance, the PHP manual mentions that the slim wrapper imagedashedline() is deprecated (although, calling the function does not raise a deprecation warning).

vapier commented 6 years ago

let's deprecate it properly

cmb69 commented 6 years ago

let's deprecate it properly

Would a respective note in the documentation suffice, or should we also emit a GD_NOTICE or even a GD_WARNING?

vapier commented 6 years ago

GD_NOTICE sounds fine

we should also be able to tag the func decl as deprecated. that way anyone compiling against it will get a build-time warning.

maybe something like (untested):

#if defined(__GNUC__) || defined(__clang__)
# define BGD_ATTR_DEPRECATED __attribute__((__deprecated__))
#else
# define BGD_ATTR_DEPRECATED
#endif
...
BGD_DECLARE(void) gdImageDashedLine (gdImagePtr im, int x1, int y1, int x2, int y2,
                                     int color) BGD_ATTR_DEPRECATED;
cmb69 commented 6 years ago

Thanks, I like that! However, there are two issues. Most importantly, if we'd ever have a test for gdImageDashedLine() make check would fail due to the deprecation warning which is elevated to an error. Secondly, while MSVC supports __declspec(deprecated), it does not support __declspec at the end of the declaration. So maybe we could do something like this (barely tested):

 src/gd.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/gd.h b/src/gd.h
index a81af1f..eb8332c 100644
--- a/src/gd.h
+++ b/src/gd.h
@@ -76,7 +76,16 @@ extern "C" {
 # define BGD_STDCALL
 #endif

+#if defined(__GNUC__) || defined(__clang__)
+# define BGD_ATTR_DEPRECATED __attribute__((__deprecated__))
+#elif defined(_MSC_VER)
+# define BGD_ATTR_DEPRECATED __declspec(deprecated)
+#else
+# define BGD_ATTR_DEPRECATED
+#endif
+
 #define BGD_DECLARE(rt) BGD_EXPORT_DATA_PROT rt BGD_STDCALL
+#define BGD_DEPRECATED(rt) BGD_EXPORT_DATA_PROT BGD_ATTR_DEPRECATED rt BGD_STDCALL

 /* VS2012+ disable keyword macroizing unless _ALLOW_KEYWORD_MACROS is set
    We define inline, snprintf, and strcasecmp if they're missing 
@@ -726,7 +735,7 @@ BGD_DECLARE(void) gdImageLine (gdImagePtr im, int x1, int y1, int x2, int y2, in

 /* For backwards compatibility only. Use gdImageSetStyle()
    for much more flexible line drawing. */
-BGD_DECLARE(void) gdImageDashedLine (gdImagePtr im, int x1, int y1, int x2, int y2,
+BGD_DEPRECATED(void) gdImageDashedLine (gdImagePtr im, int x1, int y1, int x2, int y2,
                                      int color);
 /* Corners specified (not width and height). Upper left first, lower right
    second. */
vapier commented 6 years ago

we can prob add an internal define/check for test logic to disable all deprecated warnings

willson-chen commented 5 years ago

@cmb69 hi, I am trying to add test cases for some APIs, while gdImageDashedLine is one of them. Your patch looks good. Could you please merge it to libgd-master? Or should I create a PR to submit this patch if you don't mind?