Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C99 Infinity problem #16

Open
obuchtala opened this issue Sep 4, 2013 · 8 comments
Open

C99 Infinity problem #16

obuchtala opened this issue Sep 4, 2013 · 8 comments

Comments

@obuchtala
Copy link
Owner

Taken from https://github.com/oliver----/swig-v8/issues/12#issuecomment-23750151:

I just hit a new problem with the use of C99 INFINITY. I
have APIs that expect a float, so when the SWIG API is called to
convert double to float, an overflow check is run and my code throws
an error.

SWIGINTERN int
SWIG_AsVal_float SWIG_JSC_AS_DECL_ARGS(JSValueRef obj, float *val)
{
double v;
int res = SWIG_AsVal_double SWIG_JSC_AS_CALL_ARGS(obj, &v);
if (SWIG_IsOK(res)) {
if ((v < -FLT_MAX || v > FLT_MAX)) {
return SWIG_OverflowError;
} else {
if (val) *val = (float)(v);
}
}
return res;
}

If I remove the overflow check, my code seems to work.

@obuchtala
Copy link
Owner Author

That is a UTL related issue. Probably I have missed something there when activating UTL support.

@ewmailing could you add/activate a test-case for that?

@obuchtala
Copy link
Owner Author

Hmmm. That's the JSC part... I have to make the JSC tests running first.

@ewmailing
Copy link

On 9/4/13, Oliver Buchtala [email protected] wrote:

That is a UTL related issue. Probably I have missed something there when
activating UTL support.

@ewmailing could you add/activate a test-case for that?

Sure. Though maybe you can tell me if I'm doing this correctly.

I created an infinity.i file in Examples/test-suite:

%module infinity
#include <math.h>
// C99 defines INFINITY
#define MYINFINITY (1e1000)
%inline %{
// Use of float is intentional because the original bug was in the
float conversion
float use_infinity(float inf_val)
{
return inf_val;
}
%}

Then in Examples/test-suite/javascript, I created infinity_runme.js:
var my_infinity = infinity.MYINFINTY;
var ret_val = infinity.use_infinity(my_infinity);

Is that correct?

Also, what is the correct way to expose conditional #defines like
INFINITY? My above case cheats.

I wanted to do something like this, but it doesn't work and nothing
gets exposed to SWIG:
#include <math.h>
#define MYINFINITY INFINITY
where INFINITY is supplied by <math.h> in C99.

The header file that this case originated from looks like this to
handle non-C99 cases:
#ifndef INFINITY
#ifdef _MSC_VER
union MSVC_EVIL_FLOAT_HACK
{
unsigned __int8 Bytes[4];
float Value;
};
static union MSVC_EVIL_FLOAT_HACK INFINITY_HACK = {{0x00, 0x00, 0x80, 0x7F}};
#define INFINITY (INFINITY_HACK.Value)
#endif

#ifdef __GNUC__
    #define INFINITY (__builtin_inf())
#elif defined(__clang__)
    #if __has_builtin(__builtin_inf)
        #define INFINITY (__builtin_inf())
    #endif      
#endif

#ifndef INFINITY
    #define INFINITY (1e1000)
#endif

#endif

SWIG always hits the fallback case of 1e1000 and uses it directly
without conditionals. I would like this to be more correct and
encapsulate all the conditional logic if possible.

Thanks,
Eric

@ewmailing
Copy link

New information:

  • This affects v8 too
  • This affects Python in the official 2.0.10 release.

I think this is a general SWIG bug.

Still would appreciate comments if my unit test was correct and how to
handle the #define for INFINITY.

Thanks,
Eric

@obuchtala
Copy link
Owner Author

Sorry, I didn't get it fully.

Is it so, that INFINITY gets defined only in C99 compatible math.hs?
Would it work to add an %inline block with an #ifndef INFINITY guarded #define INFINITY?

Those things should probably go into Lib/math.i.

@obuchtala
Copy link
Owner Author

Eric, could you push your new test into a branch in your repos?

@ewmailing
Copy link

On 9/5/13, Oliver Buchtala [email protected] wrote:

Eric, could you push your new test into a branch in your repos?


Reply to this email directly or view it on GitHub:
https://github.com/oliver----/swig-v8/issues/16#issuecomment-23855720

I still haven't gotten through the rebasing stuff and I'm worried my
branch is going to ultimately be incompatible with either your stuff
or the people on my team that are using my branch.

So for the meantime, I made at new branch called js_unit_tests for my fork at
https://github.com/ewmailing/swig-v8
This one is derived from you current devel branch.

It contains a new test called infinity.i which triggers both the
overflow problem and the new issue #18 I created (%rename is broken).

@obuchtala
Copy link
Owner Author

Great, thanks.

obuchtala pushed a commit that referenced this issue Sep 27, 2013
Prefer $MAKE if specified in environment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants