- Fixed point to floating point conversion is unimplemented.
- If one of the operands has a floating type and the other operand has a fixed-point type, the function handleFloatConversion() is called because one of the operands has a floating type, but we do not handle fixed point type in this function (Implementation of fixed point to floating point conversion is missing), due to this compiler crashes. In order to avoid compiler crash, when one of the operands has a floating type and the other operand has a fixed-point type, return NULL.
- FIXME: Implementation of fixed point to floating point conversion.
- I am going to resolve FIXME in followup patches.
- Add the test case.
I think a more apt subject would be "[clang] Handle floating point to fixed-point conversion crash." The commit message should also refer back to the original Bugzilla ticket.
A test case which exhibits the behavior should also be added. test/Frontend/fixed_point_errors.c is probably a good place for that.
As I mentioned per email, this is not quite right. Floating-fixedpoint conversion is simply unimplemented.
The comment should probably contain a FIXME referring to this.
I think this check would be better placed inside handleFloatConversion.
Or is it necessary that it is checked before handleComplexFloatConversion?
Does this pass? This should be producing an error, which -verify will choke on. You'll need an expected-error comment similar to the other ones in the file.
Let me know if I incorrectly interpreted it.
The patch looks good now, if you can provide a more descriptive summary/commit message, I can commit it for you.
This is correct, but if diagnostics tests like fixed_point_errors.c produce diagnostics that aren't described in the file, the test will fail.
It looks good now.
I don't think "Handle fixed point conversion" is a descriptive enough title.
I am going to resolve FIXME in followup patches.
I assume by this you mean to move forward with adding floating-fixed conversions? I have an unsubmitted patch for APFixedPoint which implements the semantics of the conversions, but I'm currently waiting on D85312, D85314, and more recently I found D54749 which is probably a prerequisite for the IR codegen.
Alright. There's a lot more that needs to be done to get the conversions to work, though. I would recommend having a look at some of the earlier patches, like D56900. The approach for codegen will probably be different, though. There's also the patches I mentioned earlier for moving all of the codegen+APFixedPoint code to LLVM, so things may move around in the future.
I can upload my partial APFixedPoint patch as a reference of the semantics of the conversions.