[cfe-commits] [PATCH, RFC] Fix PR14881 (complex integer promotion rules)

[John McCall]

On Jan 16, 2013, at 1:41 PM, Bill Schmidt <wschmidt at linux.vnet.ibm.com> wrote:
> This patch addresses PR14881.
> 
> Prior to the patch, Clang does not properly promote types when a complex
> integer operand is combined with an integer via a binary operator, or when
> one is assigned to the other in either order.  This patch detects when
> promotion is needed (and permissible) and generates the necessary code.
> 
> I've included a rather extensive test case to exercise all the various
> cases, including signed and unsigned variants.  I believe that the necessary
> handling of signedness is already in place, and the generated code confirms
> this to my best understanding.  I hope that the test will run on all targets.
> I've assumed that no target has the same size operands for "char" and
> "long long," and that no target performs arithmetic on char operands without
> extending them to a larger format first.  If there are any targets for which
> this is not the case, the test case will need adjustment.  I'd appreciate
> any information about the validity of my assumptions before I turn the test
> loose on the world...
> 
> Thanks in advance for your review!

Minor point:
+    const QualType LHSBaseType = LHSComplexInt->getElementType();
We don't usually make local variables 'const' like this.  It looks like you
don't, either, except that you want to do it to QualType.  QualType is a very
small, cheap-to-copy value type — essentially a pointer — and you should
treat it like any other type with those properties.

Major point:  it doesn't look like you're considering signedness at all as
part of your promotions.  C wants us to do the usual arithmetic conversions
on the corresponding real type in order to select the common real type;
maybe you can extract those out from handleIntegerConversion somehow?

John.