On Feb 27, 2012, at 6:21 AM, Benjamin Kramer wrote:

> 
> On 26.02.2012, at 23:15, Eli Friedman wrote:
> 
>> On Sun, Feb 26, 2012 at 8:55 AM, Benjamin Kramer
>> <[email protected]> wrote:
>>> Author: d0k
>>> Date: Sun Feb 26 10:55:55 2012
>>> New Revision: 151494
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=151494&view=rev
>>> Log:
>>> Bit pack ExtProtoInfo.
>>> 
>>> Modified:
>>>   cfe/trunk/include/clang/AST/Type.h
>>> 
>>> Modified: cfe/trunk/include/clang/AST/Type.h
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=151494&r1=151493&r2=151494&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/AST/Type.h (original)
>>> +++ cfe/trunk/include/clang/AST/Type.h Sun Feb 26 10:55:55 2012
>>> @@ -2699,15 +2699,15 @@
>>>  struct ExtProtoInfo {
>>>    ExtProtoInfo() :
>>>      Variadic(false), HasTrailingReturn(false), ExceptionSpecType(EST_None),
>>> -      TypeQuals(0), RefQualifier(RQ_None), NumExceptions(0), Exceptions(0),
>>> +      RefQualifier(RQ_None), TypeQuals(0), NumExceptions(0), Exceptions(0),
>>>      NoexceptExpr(0), ConsumedArguments(0) {}
>>> 
>>>    FunctionType::ExtInfo ExtInfo;
>>> -    bool Variadic;
>>> -    bool HasTrailingReturn;
>>> -    ExceptionSpecificationType ExceptionSpecType;
>>> +    bool Variadic : 1;
>>> +    bool HasTrailingReturn : 1;
>>> +    ExceptionSpecificationType ExceptionSpecType : 4;
>>> +    RefQualifierKind RefQualifier : 2;
>> 
>> Are you sure this won't cause issues with MSVC?  It handles enums in
>> bitfields in a non-standard way…
> 
> The bit fields should be wide enough not to be affected by MSVC's sign 
> extension weirdness. And the buildbot is still green.
> 
> However, we can revert this change if something pops up, the size of this 
> struct isn't critical.

RefQualifierKind RefQualifier : 2
has enumerator that touches the high bit so this is already problematic for 
MSVC.

ExceptionSpecificationType ExceptionSpecType : 4
does not have an enumerator that touches the hight bit currently but there may 
be more enumerators in the future or someone may reduce the bitfield size, and 
then the fields are going to be used in a certain way that will fail in MSVC 
and we will be left scratching our heads.

Please change the bitfield enums to 'unsigned'.

Also consider adding getter/setters to convert from/to unsigned <-> enum.

> 
> - Ben
>> 
>> -Eli
> 
> 
> _______________________________________________
> 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

Reply via email to