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

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

Open
dscottboggs opened this issue Dec 27, 2018 · 6 comments

Comments

@dscottboggs
Copy link
Contributor

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

@kalinon
Copy link
Contributor

kalinon commented Sep 2, 2019

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

@dscottboggs
Copy link
Contributor Author

I see, thank you!

@kalinon
Copy link
Contributor

kalinon commented May 8, 2020

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.

@docelic
Copy link
Collaborator

docelic commented May 8, 2020

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

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

@kalinon
Copy link
Contributor

kalinon commented May 8, 2020

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

@docelic
Copy link
Collaborator

docelic commented May 12, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants