From: John McCall Date: Tue, 8 Feb 2011 22:35:49 +0000 (+0000) Subject: When checking the 'weak' and 'weakref' attributes, look for non-external X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=332bb2a2e3cd0a5af85758847a8050ae8ceee5f3;p=clang When checking the 'weak' and 'weakref' attributes, look for non-external linkage rather than the presence of the 'static' storage class specifier. Fixes rdar://problem/8814626. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@125126 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index db83fbb824..05905c23f1 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1051,11 +1051,11 @@ def warn_attribute_weak_on_local : Warning< def warn_weak_identifier_undeclared : Warning< "weak identifier %0 never declared">; def err_attribute_weak_static : Error< - "weak declaration of '%0' must be public">; + "weak declaration cannot have internal linkage">; def warn_attribute_weak_import_invalid_on_definition : Warning< "'weak_import' attribute cannot be specified on a definition">; def err_attribute_weakref_not_static : Error< - "weakref declaration of '%0' must be static">; + "weakref declaration must have internal linkage">; def err_attribute_weakref_not_global_context : Error< "weakref declaration of '%0' must be in a global context">; def err_attribute_weakref_without_alias : Error< diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index 08a7c49d31..3d64ade173 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -586,11 +586,23 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) { start, size)); } -static bool isStaticVarOrStaticFunction(Decl *D) { - if (VarDecl *VD = dyn_cast(D)) - return VD->getStorageClass() == SC_Static; - if (FunctionDecl *FD = dyn_cast(D)) - return FD->getStorageClass() == SC_Static; +/// Whether this declaration has internal linkage for the purposes of +/// things that want to complain about things not have internal linkage. +static bool hasEffectivelyInternalLinkage(NamedDecl *D) { + switch (D->getLinkage()) { + case NoLinkage: + case InternalLinkage: + return true; + + // Template instantiations that go from external to unique-external + // shouldn't get diagnosed. + case UniqueExternalLinkage: + return true; + + case ExternalLinkage: + return false; + } + llvm_unreachable("unknown linkage kind!"); return false; } @@ -601,6 +613,14 @@ static void HandleWeakRefAttr(Decl *d, const AttributeList &Attr, Sema &S) { return; } + if (!isa(d) && !isa(d)) { + S.Diag(Attr.getLoc(), diag::err_attribute_wrong_decl_type) + << Attr.getName() << 2 /*variables and functions*/; + return; + } + + NamedDecl *nd = cast(d); + // gcc rejects // class c { // static int a __attribute__((weakref ("v2"))); @@ -614,7 +634,7 @@ static void HandleWeakRefAttr(Decl *d, const AttributeList &Attr, Sema &S) { const DeclContext *Ctx = d->getDeclContext()->getRedeclContext(); if (!Ctx->isFileContext()) { S.Diag(Attr.getLoc(), diag::err_attribute_weakref_not_global_context) << - dyn_cast(d)->getNameAsString(); + nd->getNameAsString(); return; } @@ -636,9 +656,8 @@ static void HandleWeakRefAttr(Decl *d, const AttributeList &Attr, Sema &S) { // This looks like a bug in gcc. We reject that for now. We should revisit // it if this behaviour is actually used. - if (!isStaticVarOrStaticFunction(d)) { - S.Diag(Attr.getLoc(), diag::err_attribute_weakref_not_static) << - dyn_cast(d)->getNameAsString(); + if (!hasEffectivelyInternalLinkage(nd)) { + S.Diag(Attr.getLoc(), diag::err_attribute_weakref_not_static); return; } @@ -1274,28 +1293,28 @@ static void HandleWarnUnusedResult(Decl *D, const AttributeList &Attr, Sema &S) D->addAttr(::new (S.Context) WarnUnusedResultAttr(Attr.getLoc(), S.Context)); } -static void HandleWeakAttr(Decl *D, const AttributeList &Attr, Sema &S) { +static void HandleWeakAttr(Decl *d, const AttributeList &attr, Sema &S) { // check the attribute arguments. - if (Attr.getNumArgs() != 0) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0; + if (attr.getNumArgs() != 0) { + S.Diag(attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0; return; } - /* weak only applies to non-static declarations */ - if (isStaticVarOrStaticFunction(D)) { - S.Diag(Attr.getLoc(), diag::err_attribute_weak_static) << - dyn_cast(D)->getNameAsString(); + if (!isa(d) && !isa(d)) { + S.Diag(attr.getLoc(), diag::warn_attribute_wrong_decl_type) + << attr.getName() << 2 /*variables and functions*/; return; } - // TODO: could also be applied to methods? - if (!isa(D) && !isa(D)) { - S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) - << Attr.getName() << 2 /*variable and function*/; + NamedDecl *nd = cast(d); + + // 'weak' only applies to declarations with external linkage. + if (hasEffectivelyInternalLinkage(nd)) { + S.Diag(attr.getLoc(), diag::err_attribute_weak_static); return; } - D->addAttr(::new (S.Context) WeakAttr(Attr.getLoc(), S.Context)); + nd->addAttr(::new (S.Context) WeakAttr(attr.getLoc(), S.Context)); } static void HandleWeakImportAttr(Decl *D, const AttributeList &Attr, Sema &S) { diff --git a/test/Sema/attr-weak.c b/test/Sema/attr-weak.c index 2abe0682df..41c9fd7165 100644 --- a/test/Sema/attr-weak.c +++ b/test/Sema/attr-weak.c @@ -11,5 +11,4 @@ void __attribute__((weak_import)) g5(void) { struct __attribute__((weak)) s0 {}; // expected-warning {{'weak' attribute only applies to variables and functions}} struct __attribute__((weak_import)) s1 {}; // expected-warning {{'weak_import' attribute only applies to variables and functions}} -static int x __attribute__((weak)); // expected-error {{weak declaration of 'x' must be public}} - +static int x __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}} diff --git a/test/SemaCXX/attr-weak.cpp b/test/SemaCXX/attr-weak.cpp new file mode 100644 index 0000000000..b6a9e0aa84 --- /dev/null +++ b/test/SemaCXX/attr-weak.cpp @@ -0,0 +1,29 @@ +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -verify %s + +static int test0 __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}} +static void test1() __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}} + +namespace test2 __attribute__((weak)) { // expected-warning {{'weak' attribute only applies to variables and functions}} +} + +namespace { + int test3 __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}} + void test4() __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}} +} + +struct Test5 { + static void test5() __attribute__((weak)); // no error +}; + +namespace { + struct Test6 { + static void test6() __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}} + }; +} + +template struct Test7 { + void test7() __attribute__((weak)) {} +}; +namespace { class Internal; } +template struct Test7; +template struct Test7; diff --git a/test/SemaCXX/attr-weakref.cpp b/test/SemaCXX/attr-weakref.cpp index 11368d9399..a34579198f 100644 --- a/test/SemaCXX/attr-weakref.cpp +++ b/test/SemaCXX/attr-weakref.cpp @@ -24,8 +24,8 @@ class c { static int a __attribute__((weakref ("v2"))); // expected-error {{declaration of 'a' must be in a global context}} static int b() __attribute__((weakref ("f3"))); // expected-error {{declaration of 'b' must be in a global context}} }; -int a7() __attribute__((weakref ("f1"))); // expected-error {{declaration of 'a7' must be static}} -int a8 __attribute__((weakref ("v1"))); // expected-error {{declaration of 'a8' must be static}} +int a7() __attribute__((weakref ("f1"))); // expected-error {{weakref declaration must have internal linkage}} +int a8 __attribute__((weakref ("v1"))); // expected-error {{weakref declaration must have internal linkage}} // gcc accepts this -int a9 __attribute__((weakref)); // expected-error {{declaration of 'a9' must be static}} +int a9 __attribute__((weakref)); // expected-error {{weakref declaration must have internal linkage}}