[cfe-commits] r169041 - /cfe/trunk/lib/CodeGen/CGExpr.cpp

[Aaron Ballman]

I'd be fine with the cast if it's more desirable.  I figured the ?:
was more clear as to the values for those unfamiliar with the integer
promotion rules, but am not strongly tied to it.

~Aaron

On Fri, Nov 30, 2012 at 5:42 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Fri, Nov 30, 2012 at 2:24 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> In theory, yes we should be able to drop the conditional operator
>> since bool is integer promoted to 0 or 1.  But MSVC warns about the
>> unsafe mixture, hence the explicit conditional operator. Ironically
>> enough, MSVC doesn't warn about the order of operations bug.  ;-)
>
> An explicit cast fixes the MSVC warning too, no? (& is probably nice
> for readability anyway - the conditional operator on the other hand is
> a lot more to parse, visually, I think)
>
> That being said, I'm not terribly fussed either way - just thinking aloud.
>
>>
>> ~Aaron
>>
>> On Fri, Nov 30, 2012 at 5:21 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>> On Fri, Nov 30, 2012 at 1:44 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>>> Author: aaronballman
>>>> Date: Fri Nov 30 15:44:01 2012
>>>> New Revision: 169041
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=169041&view=rev
>>>> Log:
>>>> Fixing a precedence issue with my previous commit.
>>>>
>>>> Modified:
>>>>     cfe/trunk/lib/CodeGen/CGExpr.cpp
>>>>
>>>> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=169041&r1=169040&r2=169041&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
>>>> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Nov 30 15:44:01 2012
>>>> @@ -1950,7 +1950,7 @@
>>>>    if (T->isIntegerType()) {
>>>>      TypeKind = 0;
>>>>      TypeInfo = (llvm::Log2_32(getContext().getTypeSize(T)) << 1) |
>>>> -               T->isSignedIntegerType() ? 1 : 0;
>>>> +               (T->isSignedIntegerType() ? 1 : 0);
>>>
>>> Should you just drop the conditional operator entirely?
>>> (should we have another class of -Wparentheses warning to catch this?)
>>>
>>>>    } else if (T->isFloatingType()) {
>>>>      TypeKind = 1;
>>>>      TypeInfo = getContext().getTypeSize(T);
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits