From be4be16be3f358e08cba4676f057e46d51e08297 Mon Sep 17 00:00:00 2001 From: Hans Wennborg <hans@hanshq.net> Date: Fri, 23 May 2014 20:16:41 +0000 Subject: [PATCH] Don't suppress warning about dllimport on typedefs etc. in MicrosoftExt mode It's true the MSVC doesn't warn about dllimport when applied to e.g. a typedef, but that applies to dllexport too. I'd like us to be consistent, and I think the right thing to do is to warn. The original test that came with implementing the old behaviour doesn't provide a good motivation, and it said it was checking that we're not repoting an *error*, which is still true since this is just a warning. There are plenty of tests e.g. in Sema/dllimport.c to check that we do warn about dllimport on non functions or variables. Differential Revision: http://reviews.llvm.org/D3832 llvm-svn: 209546 --- clang/include/clang/Basic/Attr.td | 3 +-- clang/lib/Sema/SemaDeclAttr.cpp | 12 ------------ clang/test/Rewriter/dllimport-typedef.c | 17 ----------------- 3 files changed, 1 insertion(+), 31 deletions(-) delete mode 100644 clang/test/Rewriter/dllimport-typedef.c diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 1b011b06868e..ce8ca1a568f4 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1665,8 +1665,7 @@ def DLLExport : InheritableAttr, TargetSpecificAttr<TargetWindows> { def DLLImport : InheritableAttr, TargetSpecificAttr<TargetWindows> { let Spellings = [Declspec<"dllimport">, GCC<"dllimport">]; - // Technically, the subjects for DllImport are Function and Var, but there is - // custom semantic handling required when MicrosoftExt is true. + let Subjects = SubjectList<[Function, Var]>; let Documentation = [Undocumented]; } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 9775c58639c2..e579b11f1d13 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3836,18 +3836,6 @@ DLLImportAttr *Sema::mergeDLLImportAttr(Decl *D, SourceRange Range, } static void handleDLLImportAttr(Sema &S, Decl *D, const AttributeList &Attr) { - // Attribute can be applied only to functions or variables. - FunctionDecl *FD = dyn_cast<FunctionDecl>(D); - if (!FD && !isa<VarDecl>(D)) { - // Apparently Visual C++ thinks it is okay to not emit a warning - // in this case, so only emit a warning when -fms-extensions is not - // specified. - if (!S.getLangOpts().MicrosoftExt) - S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) - << Attr.getName() << ExpectedVariableOrFunction; - return; - } - unsigned Index = Attr.getAttributeSpellingListIndex(); DLLImportAttr *NewAttr = S.mergeDLLImportAttr(D, Attr.getRange(), Index); if (NewAttr) diff --git a/clang/test/Rewriter/dllimport-typedef.c b/clang/test/Rewriter/dllimport-typedef.c deleted file mode 100644 index b587f16f3e41..000000000000 --- a/clang/test/Rewriter/dllimport-typedef.c +++ /dev/null @@ -1,17 +0,0 @@ -// RUN: not %clang_cc1 -triple i686-pc-win32 -fms-extensions -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-NEG %s -// RUN: not %clang_cc1 -triple i686-pc-win32 -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-POS %s - -// Do not report an error with including dllimport in the typedef when -fms-extensions is specified. -// Addresses <rdar://problem/7653870>. -typedef __declspec(dllimport) int CB(void); - -// This function is added just to trigger a diagnostic. This way we can test how many -// diagnostics we expect. -void bar() { return 1; } - -// CHECK-NEG: error: void function 'bar' should not return a value -// CHECK-NEG: {{^}}1 error generated -// CHECK-POS: warning: 'dllimport' attribute only applies to variables and functions -// CHECK-POS: error: void function 'bar' should not return a value -// CHECK-POS: 1 warning and 1 error generated - -- GitLab