[PATCH] D45383: Strip reference from a va_list object in C when merging parameter types.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D45383#1061797, @efriedma wrote:

> > vprintf is handled using the exact same code-paths. SO, it'll have its 2nd parameter created with type 'char*&'
>
> vprintf is defined in the C standard as "int vprintf(const char *format, va_list arg);"; on Windows, that's equivalent to "int vprintf(const char *format, char* arg);".  If a reference is showing up anywhere, something has gone seriously wrong before we get anywhere near the type merging code.


Hrm... looking back again, I believe you're right.  I did a more careful grepping and found that only 4 builtins and 3 C99 library functions have this issue.  Basically, anything with a 'A' in the parameter spec in Builtins.def.  I don't know how I got attached to vprintf, I apologize for the wasted time.

The functions with this problem are:
__builtins_va_start, __builtins_va_end, __builtins_va_copy, and __builtin_stdarg_start. 
va_start, va_end, va_copy.

I don't see redeclaring the latter 3 as being disallowed, so I presume that we'd not be able to prevent that.  I'm really open to suggestions, as thinking over the weekend has convinced me that this patch is a little bit hacky.  I'd like some solution for the assertion, but I'm not sure I see one.  I note that godbolt (https://godbolt.org/g/qkryPR) doesn't show the assert as it is a non-assert build.  Would we be OK with simply relaxing the '!isreferencetype' assert to something like, "!isReferenceType || isVaListTypeRef"?


Repository:
  rC Clang

https://reviews.llvm.org/D45383