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

[Enea Zaffanella]

On 11/29/2012 05:09 PM, Rafael Espindola wrote:
> Author: rafael
> Date: Thu Nov 29 10:09:03 2012
> New Revision: 168895
>
> URL: http://llvm.org/viewvc/llvm-project?rev=168895&view=rev
> Log:
> Merge function types in C.


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?

Enea.