[cfe-dev] clang::Qualifiers::TQ flags are not in sync with DeclSpec::TQ flags

[John McCall via cfe-dev]

> On Aug 10, 2017, at 11:31 AM, Vladimir Voskresensky <vladimir.voskresensky at oracle.com> wrote:
> On 10.08.2017 18:19, John McCall wrote:
>>> On Aug 10, 2017, at 7:56 AM, Vladimir Voskresensky <vladimir.voskresensky at oracle.com> wrote:
>>> On 10.08.2017 02:31, John McCall wrote:
>>>>> On Aug 9, 2017, at 4:51 PM, Vladimir Voskresensky via cfe-dev <cfe-dev at lists.llvm.org> wrote:
>>>>> Ping,
>>>>> 
>>>>> Could someone comment, please, if clang::Qualifiers::TQ intentionally reduces number of flags from 5 to 3?
>>>> It is not intentional, no.  We do not represent _Atomic as an ordinary qualifier in the AST, so I think this might accidentally be okay there.  Offhand, I don't think we want to support either of those qualifiers as an array index type qualifier unless the C spec absolutely demands it.
>>> Ok. Thanks for the information. I was mislead by
>>> comment in clang::Qualifiers::TQ
>>> // NOTE: These flags must be kept in sync with DeclSpec::TQ.
>>> and the test where 5 bits value was written into 3 bits field.
>>> 
>>> Would be great if "note" can be clarified as well.
>> If something is just blindly trying to propagate the DeclSpec::TQ around, that seems problematic.
> If only 3 flags are really wanted here, then probably this line
> 
> ArrayTypeBits.IndexTypeQuals = tq;
> 
> should use cleaning mask:
> ArrayTypeBits.IndexTypeQuals = tq & Qualifiers::CVRMask;

I think that would be an excellent clarification in the code, yeah.

John.

> 
> Vladimir.
>> 
>> I think the comment is more about the flag values than the full set of flags, but yes, that should be clarified.
>> 
>> John.
>> 
>>> Thanks
>>> Vladimir.
>>> 
>>>> John.
>>>> 
>>>>> Thanks,
>>>>> Vladimir.
>>>>> 
>>>>> On 07.08.2017 16:06, Vladimir Voskresensky via cfe-dev wrote:
>>>>>> Hello,
>>>>>> 
>>>>>> clang::Qualifiers::TQ has 3 flags and CVRMask mask. Also it has the note:
>>>>>> // NOTE: These flags must be kept in sync with DeclSpec::TQ.
>>>>>> 
>>>>>> But
>>>>>> DeclSpec::TQ has 5 flags (so  TQ_unaligned = 8, TQ_atomic = 16 are missed in clang::Qualifiers::TQ)
>>>>>> 
>>>>>> This inconsistency can be found by debugging test llvm/tools/clang/test/Sema/MicrosoftExtensions.c on the last declaration:
>>>>>> void test_unaligned2(int x[__unaligned 4]) {}
>>>>>> 
>>>>>> Array type is created incorrectly here, because '__unaligned' is lost in the ArrayType constructor at line:
>>>>>> ArrayTypeBits.IndexTypeQuals = tq;
>>>>>> tq has value 8 (TQ_unaligned) while IndexTypeQuals is declared to use ":3" bits only, so IndexTypeQuals is zero after assign.
>>>>>> 
>>>>>> Usually TQ fields are 5 bits (see i.e. DeclSpec::TypeQualifiers), so
>>>>>> declaration Type::ArrayTypeBitfields::IndexTypeQuals must be fixed as well
>>>>>> 
>>>>>> Hope it helps,
>>>>>> Vladimir.
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> _______________________________________________
>>>>>> cfe-dev mailing list
>>>>>> cfe-dev at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>> _______________________________________________
>>>>> cfe-dev mailing list
>>>>> cfe-dev at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>