[cfe-commits] Source code fidelity problem after commit 168895.

[Enea Zaffanella]

On 01/03/2013 04:16 AM, Rafael Espíndola wrote:
>> After this commit, for the following code
>>
>> $ cat merge.c
>> typedef int A;
>> A f();
>> typedef int B;
>> B f();
>>
>> we obtain the following AST dump, where typedef B gets replaced by A in the
>> second declaration of f():
>>
>> $ clang -cc1 -ast-dump merge.c
>> [...]
>>    (TypedefDecl 0x4e1f220 <merge.c:1:1, col:13> A 'int')
>>    (FunctionDecl 0x4e1f310 <line:2:1, col:5> f 'A ()')
>>    (TypedefDecl 0x4e1f3c0 <line:3:1, col:13> B 'int')
>>    (FunctionDecl 0x4e1f480 <line:4:1, col:5> f 'A ()'))
>>
>>
>> The chunk responsible for this problem seems to be the following:
>>
>>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=168895&r1=168894&r2=168895&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Nov 29 10:09:03 2012
>>> @@ -2402,6 +2402,12 @@
>>>      if (getLangOpts().CPlusPlus)
>>>        return MergeCXXFunctionDecl(New, Old, S);
>>>
>>> +  // Merge the function types so the we get the composite types for the
>>> return
>>> +  // and argument types.
>>> +  QualType Merged = Context.mergeTypes(Old->getType(), New->getType());
>>> +  if (!Merged.isNull())
>>> +    New->setType(Merged);
>>> +
>>>      return false;
>>>    }
>>
>>
>> As a bare minimum, I'd suggest to exchange the order of arguments in the
>> call to mergeTypes():
>>
>> -  QualType Merged = Context.mergeTypes(Old->getType(), New->getType());
>> +  QualType Merged = Context.mergeTypes(New->getType(), Old->getType());
>>
>> This will fix (just) the test case above ...
>> however, in general, what happens to the *syntactic* type after merging?
>> Is it going to be simply discarded (i.e., replaced by the merged one)?
>> If so, what about source code fidelity?
>
> That is a good question. I noticed that clang was merging some of the
> types, but not all. Subsequent passes of clang expect the merged type,
> so merging the missing cases seemed the right answer.
>
> So in general clang doesn't keep the "type as written". It should be
> possible to always keep that, and have getType() do the merging, but
> that is probably really expensive, which I assume is why we don't keep
> it. Douglas, is that the case?


mmmhhh ... I now think I may have been overlooking something.

When clang merges types, will it touch the TypeSourceInfo at all?
If that info is fully preserved, then source fidelity will be preserved 
too (as far as I remember the two methods getType and getTypeSourceInfo 
return different data for function declarations).

Enea.