[cfe-commits] r151494 - /cfe/trunk/include/clang/AST/Type.h

[Argyrios Kyrtzidis]

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
>> <benny.kra at googlemail.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits