bindgen icon indicating copy to clipboard operation
bindgen copied to clipboard

9223372036854775808 in a macro is expected to evaluate to 9223372036854775808_u64 but gets interpreted as an Int64 instead

Open dscottboggs opened this issue 7 years ago • 6 comments

There is still one issue that remains, when running the specs:

Failures:

  1) clang tool macros feature exports the macros

       At macros.10.evaluated: Expected 9223372036854775808_u64, got -9223372036854775808_i64 (ClangValidationError)
         from /spec_helper.cr:79:91 in 'check_partial_value'
         from /spec_helper.cr:64:7 in 'check_partial_value'
         from /spec_helper.cr:75:7 in 'check_partial_value'
         from spec/clang/spec_helper.cr:31:5 in 'clang_tool:macros'
         from spec/clang/macros_spec.cr:5:5 in '->'
         from /usr/share/crystal/src/spec/methods.cr:255:3 in 'it'
         from spec/clang/macros_spec.cr:4:3 in '->'
         from /usr/share/crystal/src/spec/context.cr:255:3 in 'describe'
         from /usr/share/crystal/src/spec/methods.cr:16:5 in 'describe'
         from spec/integration/spec_helper.cr:1:1 in '__crystal_main'
         from /usr/share/crystal/src/crystal/main.cr:97:5 in 'main_user_code'
         from /usr/share/crystal/src/crystal/main.cr:86:7 in 'main'
         from /usr/share/crystal/src/crystal/main.cr:106:3 in 'main'
         from __libc_start_main
         from _start
         from ???

I'm really not sure why that's happening but it could cause some nasty bugs

dscottboggs avatar Dec 27 '18 00:12 dscottboggs

I think this is an issue with the JSON parser in the tests.

Evaluated output:

{
      "name": "EVALUATE_LARGE_UINT64",
      "isFunction": false,
      "isVarArg": false,
      "arguments": [],
      "value": "9223372036854775808",
      "type": {
        "isConst": false,
        "isMove": false,
        "isReference": false,
        "isBuiltin": true,
        "isVoid": false,
        "pointer": 0,
        "baseName": "unsigned long long",
        "fullName": "unsigned long long",
        "template": null
      },
      "evaluated": 9223372036854776000
    }

Test script:

require "json"
data = %({"evaluated": 9223372036854776000})
stuff = JSON.parse(data)

if stuff["evaluated"].raw.is_a?(Int64)
	puts "its a Int64"
else
	puts "its something else"
end

Output:

its a Int64

kalinon avatar Sep 02 '19 01:09 kalinon

I see, thank you!

dscottboggs avatar Sep 04 '19 15:09 dscottboggs

So looking into this further essentially JSON does not support unsigned long long.

From the RFC

   This specification allows implementations to set limits on the range
   and precision of numbers accepted.  Since software that implements
   IEEE 754-2008 binary64 (double precision) numbers [IEEE754] is
   generally available and widely used, good interoperability can be
   achieved by implementations that expect no more precision or range
   than these provide, in the sense that implementations will
   approximate JSON numbers within the expected precision.  A JSON
   number such as 1E400 or 3.141592653589793238462643383279 may indicate
   potential interoperability problems, since it suggests that the
   software that created it expects receiving software to have greater
   capabilities for numeric magnitude and precision than is widely
   available.

So the real issue here is that bindgen is passing the uint64 via JSON. which is not working. we will need a custom parser.

@docelic just so you know this is a blocker to some of the spec tests.

kalinon avatar May 08 '20 19:05 kalinon

Hey, yes, in terms of just passing the spec, @ZaWertun applied this hack in his branch:

 ...
-evaluated: NUMBERu64
+evaluated: -NUMBERi64
 ...

docelic avatar May 08 '20 20:05 docelic

Maybe i will have to do that. i have only a few broken specs left

kalinon avatar May 08 '20 20:05 kalinon

The master branch now contains a workaround, but needs better fix.

The code block which is part of this is:

+      value = v.getZExtValue();
+      // FIXME: Perhaps we need to convert it to string because JSON does not support uint64?
+      // Then translate it on the other end?
+      // value = std::to_string(v.getZExtValue());

Mentioning it here so that we can close ticket #26.

docelic avatar May 12 '20 11:05 docelic