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

[Rafael Espíndola]

> 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?

> Enea.
>

Cheers,
Rafael