r290146 - Fix completely bogus types for some builtins:
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 20 18:56:13 PST 2016
On 20 December 2016 at 18:14, Richard Smith <richard at metafoo.co.uk> wrote:
> On 19 December 2016 at 17:00, Friedman, Eli via cfe-commits <
> 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/vfpr
>>> intf-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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161220/9e645347/attachment.html>
-------------- next part --------------
Index: lib/AST/ASTContext.cpp
===================================================================
--- lib/AST/ASTContext.cpp (revision 290192)
+++ lib/AST/ASTContext.cpp (working copy)
@@ -8690,6 +8690,10 @@
unsigned *IntegerConstantArgs) const {
const char *TypeStr = BuiltinInfo.getTypeString(Id);
+ FunctionType::ExtInfo EI(CC_C);
+ if (BuiltinInfo.isNoReturn(Id))
+ EI = EI.withNoReturn(true);
+
SmallVector<QualType, 8> ArgTypes;
bool RequiresICE = false;
@@ -8703,8 +8707,18 @@
while (TypeStr[0] && TypeStr[0] != '.') {
QualType Ty = DecodeTypeFromStr(TypeStr, *this, Error, RequiresICE, true);
- if (Error != GE_None)
- return QualType();
+ if (Error != GE_None) {
+ if (getLangOpts().CPlusPlus || StringRef(TypeStr).back() == '.' ||
+ !BuiltinInfo.isPredefinedLibFunction(Id))
+ return QualType();
+ // In C, if a non-variadic builtin library function can't be implicitly
+ // declared with the right signature (because we're missing a necessary
+ // type declaration), declare it as an unprototyped function instead, so
+ // that we can at least still add attributes to it. We should not complain
+ // about a missing #include in this case.
+ Error = GE_None;
+ return getFunctionNoProtoType(ResType, EI);
+ }
// If this argument is required to be an IntegerConstantExpression and the
// caller cares, fill in the bitmask we return.
@@ -8724,9 +8738,6 @@
assert((TypeStr[0] != '.' || TypeStr[1] == 0) &&
"'.' should only occur at end of builtin type list!");
- FunctionType::ExtInfo EI(CC_C);
- if (BuiltinInfo.isNoReturn(Id)) EI = EI.withNoReturn(true);
-
bool Variadic = (TypeStr[0] == '.');
// We really shouldn't be making a no-proto type here.
Index: test/Sema/implicit-builtin-decl.c
===================================================================
--- test/Sema/implicit-builtin-decl.c (revision 290192)
+++ test/Sema/implicit-builtin-decl.c (working copy)
@@ -56,13 +56,17 @@
void snprintf() { }
// PR8316
-void longjmp(); // expected-warning{{declaration of built-in function 'longjmp' requires inclusion of the header <setjmp.h>}}
+void longjmp();
extern float fmaxf(float, float);
struct __jmp_buf_tag {};
-void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires inclusion of the header <setjmp.h>}}
+int sigsetjmp(struct __jmp_buf_tag[1], int);
-// CHECK: FunctionDecl {{.*}} <line:[[@LINE-2]]:1, col:44> col:6 sigsetjmp '
+// CHECK: FunctionDecl {{.*}} <line:[[@LINE-2]]:5> col:5 implicit sigsetjmp 'int ()'
// CHECK-NOT: FunctionDecl
// CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Implicit
+
+// CHECK: FunctionDecl {{.*}} <col:1, col:43> col:5 sigsetjmp '
+// CHECK-NOT: FunctionDecl
+// CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Inherited Implicit
Index: test/Sema/vfprintf-invalid-redecl.c
===================================================================
--- test/Sema/vfprintf-invalid-redecl.c (revision 290192)
+++ test/Sema/vfprintf-invalid-redecl.c (working copy)
@@ -3,4 +3,4 @@
// The following declaration is not compatible with vfprintf(), but make
// sure this isn't an error: autoconf expects this to build.
-char vfprintf(); // expected-warning {{declaration of built-in function 'vfprintf'}}
+char vfprintf(); // expected-warning {{incompatible redeclaration of library function 'vfprintf'}} expected-note {{'vfprintf' is a builtin}}
Index: test/Sema/vfprintf-valid-redecl.c
===================================================================
--- test/Sema/vfprintf-valid-redecl.c (revision 290192)
+++ test/Sema/vfprintf-valid-redecl.c (working copy)
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 %s -fsyntax-only -pedantic -verify
// RUN: %clang_cc1 %s -fsyntax-only -pedantic -verify -DPREDECLARE
+// expected-no-diagnostics
#ifdef PREDECLARE
// PR16344
@@ -6,13 +7,10 @@
// 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; } // expected-warning {{requires inclusion of the header <stdio.h>}}
+int vfprintf(void) { return 0; }
#endif
// PR4290
// The following declaration is compatible with vfprintf, so we shouldn't
-// reject.
+// warn.
int vfprintf();
-#ifndef PREDECLARE
-// expected-warning at -2 {{requires inclusion of the header <stdio.h>}}
-#endif
More information about the cfe-commits
mailing list