r201981 - Sema: Simplify away one-iteration loops.

[Benjamin Kramer]

On 23.02.2014, at 22:16, Arthur O'Dwyer <arthur.j.odwyer at gmail.com> wrote:

> On Sun, Feb 23, 2014 at 6:34 AM, Benjamin Kramer
> <benny.kra at googlemail.com> wrote:
>> Author: d0k
>> Date: Sun Feb 23 08:34:50 2014
>> New Revision: 201981
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=201981&view=rev
>> Log:
>> Sema: Simplify away one-iteration loops.
>> 
>> No functionality change.
> […]
>> @@ -7381,10 +7375,7 @@ bool GetMatchingCType(
>>     return false;
>> 
>>   if (VD) {
>> -    for (specific_attr_iterator<TypeTagForDatatypeAttr>
>> -             I = VD->specific_attr_begin<TypeTagForDatatypeAttr>(),
>> -             E = VD->specific_attr_end<TypeTagForDatatypeAttr>();
>> -         I != E; ++I) {
>> +    if (TypeTagForDatatypeAttr *I = VD->getAttr<TypeTagForDatatypeAttr>()) {
>>       if (I->getArgumentKind() != ArgumentKind) {
>>         FoundWrongKind = true;
>>         return false;
> 
> FYI, this does *look* like a functionality change. Compare the loop in
> Sema::FinalizeDeclaration(); a declaration can have more than one
> instance of TypeTagForDatatypeAttr. Looking at just the first one
> *seems* like it would give different results from looking at all of
> them.
> 
> If I'm right, then a regression test should definitely be added: one
> that distinguishes the old code's behavior from the new code's
> behavior and decrees which one should be considered "correct".

The old code didn't look at all attributes either, the loop is always terminated
by the "return true;" a couple of lines below. I'm not 100% sure that there is no
case where checking multiple attributes would make sense, but now it's at
least obvious what it does.

- Ben