swig / swig

SWIG is a software development tool that connects programs written in C and C++ with a variety of high-level programming languages.
http://www.swig.org
Other
5.67k stars 1.23k forks source link

Wrapping long double constants #2964

Open ojwb opened 1 month ago

ojwb commented 1 month ago

This doesn't seem to work well in most target languages currently. Here's a testcase patch to demonstrate this:

diff --git a/Examples/test-suite/constant_expr_c.i b/Examples/test-suite/constant_expr_c.i
index f561cd86..2468972e 100644
--- a/Examples/test-suite/constant_expr_c.i
+++ b/Examples/test-suite/constant_expr_c.i
@@ -81,10 +81,15 @@ const int s9a = sizeof(-s8a);
 /* Regression test for #1917, fixed in 4.3.0. */
 #ifdef SWIGCSHARP
 %csconst(1) float_suffix_test;
+%csconst(1) longdouble_suffix_test;
 #endif
 #ifdef SWIGJAVA
 %javaconst(1) float_suffix_test;
+%javaconst(1) longdouble_suffix_test;
 #endif
 %constant const float float_suffix_test = 4.0f;
 %constant const float float_suffix_test2 = 4.0f;
 #define float_suffix_test3 4.0f
+%constant const long double longdouble_suffix_test = 4.0l;
+%constant const long double longdouble_suffix_test2 = 4.0l;
+#define longdouble_suffix_test3 4.0l

Spotted while looking at removing the code to strip l suffixes from these constants. This change doesn't seem to make the situation worse, but I'm hesitant to merge it without test coverage:

diff --git a/Source/Swig/scanner.c b/Source/Swig/scanner.c
index ae74ee82..6bf6118d 100644
--- a/Source/Swig/scanner.c
+++ b/Source/Swig/scanner.c
@@ -1140,7 +1140,6 @@ static int look(Scanner *s) {
       else if ((c == 'f') || (c == 'F')) {
    return SWIG_TOKEN_FLOAT;
       } else if ((c == 'l') || (c == 'L')) {
-   Delitem(s->text, DOH_END);
    return SWIG_TOKEN_LONGDOUBLE;
       } else {
    retract(s, 1);
@@ -1276,7 +1275,6 @@ static int look(Scanner *s) {
       else if ((c == 'f') || (c == 'F')) {
    return SWIG_TOKEN_FLOAT;
       } else if ((c == 'l') || (c == 'L')) {
-   Delitem(s->text, DOH_END);
    return SWIG_TOKEN_LONGDOUBLE;
       } else {
    retract(s, 1);
ojwb commented 1 month ago

This seems a bit of a can of worms, mostly because some (most?) target languages don't support long double or a suitable type to map it to (e.g. if a target language has an arbitrary precision type that would probably be a reasonable type to map it to).

In cases involving suffixed long double literals (e.g. 4.0l) I think this is essentially a regression I introduced in 7ba44dc62de77c23ed67ce4578e1042f7f4e292d when I fixed type handling not to treat float and long double literals as double literals. That's needed for C++ auto and decltype to work correctly, so reverting isn't the answer.

We currently have no typemaps for long double except for a Python doctype one, and adding them for all current target languages seems a bit of an involved project.

We could perhaps add long double to each double typemap more easily (and for languages without anything better to map long double to, that seems the best we can do), though we also need to ensure the typemap code handles the type not being exactly double.

As a workaround, the last part can be done already by the user via:

%apply double { long double };