On Jan 16, 2013, at 1:41 PM, Bill Schmidt <[email protected]> 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. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
