Open Bug 585857 Opened 14 years ago Updated 2 years ago

Fix CData case of ExplicitConvert

Categories

(Core :: js-ctypes, defect)

x86
macOS
defect

Tracking

()

People

(Reporter: dwitte, Unassigned)

References

(Blocks 1 open bug)

Details

jorendorff writes:

On 7/15/10 6:05 PM, David Herman wrote:
> ExplicitConvert tries ImplicitConvert first. ImplicitConvert is pretty strict about converting floating point numbers to integers; it expects only exact integers, no rounding. ExplicitConvert is less strict about converting ECMAScript doubles, but it doesn't have any additional logic to be so permissive about floating point CData.
>
> Concretely:
>
>> a = new ctypes.float32_t(2.3)
> ctypes.float32_t(2.299999952316284)
>> ctypes.uint32_t(2.3)
> ctypes.uint32_t(2)
>> ctypes.uint32_t(2.299999952316284)
> ctypes.uint32_t(2)
>> ctypes.uint32_t(a)
> typein:57: TypeError: expected type uint32_t, got ctypes.float32_t(2.299999952316284)
>
> This seems inconsistent, but I might be missing something?

That is inconsistent and should be fixed.

All you're missing is that somehow the whole time I was writing the
first draft of this spec I missed the case where the JS value being
(implicitly or explicitly) converted is itself a CData object--so the
conversion is really C-to-C, not JS-to-C. After that I never found time
to revisit those parts of the decision tree and get them Right.

So, for another example, ImplicitConverting a JS boolean to int32_t
works fine, but ImplicitConverting a ctypes.bool to int32_t fails, for
no good reason.

(One idea I rejected was making C-to-C ImplicitConvert behave just like
assignment in C, and ExplicitConvert behave like a C cast. First, I
certainly didn't want to #include the whole C spec or copy large parts
of it. Second, C allows all kinds of implicit lossy conversions. I
thought ImplicitConvert should reject those, as it does in JS-to-C cases.)

-j
Assignee: nobody → dwitte
Reassigning to nobody. If anyone wants to work on this, feel free!
Assignee: dwitte → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.