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 <[email protected]> wrote: > On Fri, Nov 30, 2012 at 2:24 PM, Aaron Ballman <[email protected]> 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 <[email protected]> wrote: >>> On Fri, Nov 30, 2012 at 1:44 PM, Aaron Ballman <[email protected]> >>> 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 >>>> [email protected] >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
