r290146 - Fix completely bogus types for some builtins:

[Friedman, Eli via cfe-commits]

On 12/20/2016 6:56 PM, Richard Smith wrote:
> On 20 December 2016 at 18:14, Richard Smith <richard at metafoo.co.uk 
> <mailto:richard at metafoo.co.uk>> wrote:
>
>     On 19 December 2016 at 17:00, Friedman, Eli via cfe-commits
>     <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>
>     wrote:
>
>         On 12/19/2016 3:59 PM, Richard Smith via cfe-commits wrote:
>
>             Author: rsmith
>             Date: Mon Dec 19 17:59:34 2016
>             New Revision: 290146
>
>             URL:
>             http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/vfprintf-valid-redecl.c?rev=290146&r1=290145&r2=290146&view=diff
>             <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/vfprintf-valid-redecl.c?rev=290146&r1=290145&r2=290146&view=diff>
>             ==============================================================================
>             --- cfe/trunk/test/Sema/vfprintf-valid-redecl.c (original)
>             +++ cfe/trunk/test/Sema/vfprintf-valid-redecl.c Mon Dec 19
>             17:59:34 2016
>             @@ -1,16 +1,18 @@
>               // RUN: %clang_cc1 %s -fsyntax-only -pedantic -verify
>               // RUN: %clang_cc1 %s -fsyntax-only -pedantic -verify
>             -DPREDECLARE
>             -// expected-no-diagnostics
>                 #ifdef PREDECLARE
>               // PR16344
>               // Clang has defined 'vfprint' in builtin list. If the
>             following line occurs before any other
>               // `vfprintf' in this file, and we
>             getPreviousDecl()->getTypeSourceInfo() on it, then we will
>               // get a null pointer since the one in builtin list
>             doesn't has valid TypeSourceInfo.
>             -int vfprintf(void) { return 0; }
>             +int vfprintf(void) { return 0; } // expected-warning
>             {{requires inclusion of the header <stdio.h>}}
>               #endif
>                 // PR4290
>               // The following declaration is compatible with
>             vfprintf, so we shouldn't
>             -// warn.
>             +// reject.
>               int vfprintf();
>             +#ifndef PREDECLARE
>             +// expected-warning at -2 {{requires inclusion of the header
>             <stdio.h>}}
>             +#endif
>
>
>         We shouldn't warn here; this declaration of vfprintf() is
>         compatible with the actual prototype.  (Granted, you can't
>         call vfprintf() without including stdio.h, but that's a
>         separate problem.)
>
>
>     I agree (but I'd note that this is a pre-existing problem for
>     other lib builtins taking a FILE*, particularly vfscanf). Perhaps
>     we could deem the builtin to have a FunctionNoProtoType type in C
>     if it's not variadic and depends on a type that is not yet declared?
>
>
> What do you think of the attached approach? I'm a little uncomfortable 
> that we no longer recognise a declaration of sigsetjmp as a builtin if 
> it has the wrong return type (and issue only a warning for that case).
>
> Alternatively, if we're OK with recognising wrongly-typed declarations 
> as library builtins in this corner case, we could just remove the 
> warning entirely.

The "right" approach is probably something a bit more invasive. We could 
wait, and attempt to recognize the builtin later, when the function is 
declared with a prototype or called.  Or we could try to 
"forward-declare" FILE (map it to some opaque type until we have an 
actual declaration we can use).

Short of that, it's probably okay to leave things as-is for now; better 
to have extra warnings in weird edge cases than to miss a warning in a 
case where it's important.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161221/a96dc3ea/attachment-0001.html>