rsmith updated this revision to Diff 298268.
rsmith marked 3 inline comments as done.
rsmith added a comment.
- Addressing review feedback from @aaron.ballman.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89212/new/
https://reviews.llvm.org/D89212
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/DeclBase.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Sema/attr-weak.c
clang/test/Sema/init.c
Index: clang/test/Sema/init.c
===================================================================
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -155,7 +155,7 @@
int PR4386_a = ((void *) PR4386_bar) != 0;
int PR4386_b = ((void *) PR4386_foo) != 0; // expected-error{{initializer element is not a compile-time constant}}
int PR4386_c = ((void *) PR4386_zed) != 0;
-int PR4386_zed() __attribute((weak));
+int PR4386_zed() __attribute((weak)); // expected-warning{{'PR4386_zed' declared weak after its first use}} expected-note {{attribute}}
// <rdar://problem/10185490> (derived from SPEC vortex benchmark)
typedef char strty[10];
Index: clang/test/Sema/attr-weak.c
===================================================================
--- clang/test/Sema/attr-weak.c
+++ clang/test/Sema/attr-weak.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only %s -triple x86_64-unknown-macosx10.3.0 -DMACOS
+// RUN: %clang_cc1 -verify -fsyntax-only %s -triple x86_64-linux-gnu -DLINUX
extern int f0() __attribute__((weak));
extern int g0 __attribute__((weak));
@@ -25,3 +26,55 @@
static void pr14946_f();
void pr14946_f() __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}}
+
+void some_function();
+
+void pr47663_a();
+void pr47663_b();
+static void pr47663_c();
+void pr47663_d();
+void pr47663_e(); // expected-warning {{declared weak after its first use}}
+void pr47663_f(); // expected-note {{possible target}}
+void pr47663_g();
+int pr47663_h;
+void pr47663_z() __attribute__((weak));
+void use() {
+ int arr_a[&pr47663_a ? 1 : -1];
+ int arr_b[&pr47663_b ? 1 : -1];
+ int arr_c[&pr47663_c ? 1 : -1];
+ int arr_d[&pr47663_d ? 1 : -1];
+ int arr_e[&pr47663_e ? 1 : -1];
+ int arr_f[&pr47663_f ? 1 : -1];
+ int arr_g[&pr47663_g ? 1 : -1];
+ int arr_h[&pr47663_h ? 1 : -1]; // expected-warning {{will always evaluate to 'true'}}
+ int arr_z[&pr47663_z ? -1 : 1];
+}
+void pr47663_a() __attribute__((weak)); // expected-warning {{declared weak after its first use}} expected-note {{'weak' attribute here}}
+void pr47663_b() __attribute__((weak_import)); // expected-warning {{declared weak after its first use}} expected-note {{'weak_import' attribute here}}
+#ifdef LINUX
+static void pr47663_c() __attribute__((weakref, alias("might_not_exist"))); // expected-warning {{declared weak after its first use}} expected-note {{'weakref' attribute here}}
+#endif
+#ifdef MACOS
+void pr47663_d() __attribute__((availability(macos, introduced=10.4))); // expected-warning {{declared weak after its first use}} expected-note {{'availability' attribute here}}
+#endif
+#pragma weak pr47663_e // expected-note {{pragma 'weak' here}}
+
+// FIXME: This should warn; see PR47796. But it actually creates a bogus
+// overload set. When this is fixed, ensure we produce the 'declared weak after
+// its first use' warning.
+#pragma weak pr47663_f = some_function // expected-note {{possible target}}
+void q() { pr47663_f; } // expected-error {{overloaded}}
+
+#pragma clang attribute push (__attribute__((weak)), apply_to = function) // expected-note {{'weak' attribute here}}
+void pr47663_g(); // expected-warning {{declared weak after its first use}}
+#pragma clang attribute pop
+extern int pr47663_h __attribute__((weak)); // expected-warning {{declared weak after its first use}} expected-note {{'weak' attribute here}}
+void pr47663_z() __attribute__((weak_import));
+
+// 'weak' on a definition means something different from 'weak' on a
+// declaration. Don't warn in that case.
+void weak_def_after_use();
+extern int weak_def_after_use_v;
+void use_wdau() { weak_def_after_use(); weak_def_after_use_v = 0; }
+__attribute__((weak)) void weak_def_after_use() {}
+__attribute__((weak)) int weak_def_after_use_v;
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8154,6 +8154,9 @@
W.setUsed(true);
if (W.getAlias()) { // clone decl, impersonate __attribute(weak,alias(...))
IdentifierInfo *NDId = ND->getIdentifier();
+ // FIXME (PR47796): Check for a previous declaration with the target name
+ // here, and build a redeclaration of it. Check whether the previous
+ // declaration is used and warn if so.
NamedDecl *NewD = DeclClonePragmaWeak(ND, W.getAlias(), W.getLocation());
NewD->addAttr(
AliasAttr::CreateImplicit(Context, NDId->getName(), W.getLocation()));
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6320,7 +6320,8 @@
}
}
-static void checkAttributesAfterMerging(Sema &S, NamedDecl &ND) {
+static void checkAttributesAfterMerging(Sema &S, NamedDecl &ND,
+ bool WillBeDefinition = false) {
// Ensure that an auto decl is deduced otherwise the checks below might cache
// the wrong linkage.
assert(S.ParsingInitForAutoVars.count(&ND) == 0);
@@ -6420,6 +6421,40 @@
}
}
}
+
+ // If the new declaration is weak and the old was already used, we might have
+ // used the non-weakness of the old declaration (specifically, the implied
+ // non-nullness of a non-weak declaration) in one of various ways (constant
+ // evaluation, code generation, diagnostic emission). But don't warn if we're
+ // introducing a weak (non-alias) definition, because a weak definition can't
+ // be null.
+ const auto *VD = dyn_cast<ValueDecl>(&ND);
+ if (VD && VD->isUsed(false) && VD->isWeak() &&
+ (VD->hasDefiningAttr() || !(getDefinition(VD) || WillBeDefinition))) {
+ const Attr *WeakA = nullptr;
+ for (const Attr *A : VD->getAttrs()) {
+ if (!isa<WeakAttr, WeakRefAttr, WeakImportAttr, AvailabilityAttr>(A))
+ continue;
+
+ if (const auto *Avail = dyn_cast<AvailabilityAttr>(A))
+ if (Avail->getAvailability(S.Context) != AR_NotYetIntroduced)
+ continue;
+
+ if (A->isInherited()) {
+ WeakA = nullptr;
+ break;
+ }
+ if (!WeakA)
+ WeakA = A;
+ }
+
+ if (WeakA) {
+ S.Diag(VD->getLocation(), diag::warn_attribute_weak_after_use) << VD;
+ S.Diag(WeakA->getLocation(), diag::note_declared_weak_here)
+ << (WeakA->getSyntax() == AttributeCommonInfo::AS_Pragma)
+ << WeakA->getSpelling();
+ }
+ }
}
static void checkDLLAttributeRedeclaration(Sema &S, NamedDecl *OldDecl,
@@ -9674,7 +9709,7 @@
}
ProcessPragmaWeak(S, NewFD);
- checkAttributesAfterMerging(*this, *NewFD);
+ checkAttributesAfterMerging(*this, *NewFD, D.isFunctionDefinition());
AddKnownFunctionAttributes(NewFD);
@@ -18245,36 +18280,47 @@
(void)ExtnameUndeclaredIdentifiers.insert(std::make_pair(Name, Attr));
}
-void Sema::ActOnPragmaWeakID(IdentifierInfo* Name,
+void Sema::ActOnPragmaWeakID(IdentifierInfo *Name,
SourceLocation PragmaLoc,
SourceLocation NameLoc) {
- Decl *PrevDecl = LookupSingleName(TUScope, Name, NameLoc, LookupOrdinaryName);
+ WeakInfo W((IdentifierInfo*)nullptr, NameLoc);
- if (PrevDecl) {
- PrevDecl->addAttr(WeakAttr::CreateImplicit(Context, PragmaLoc, AttributeCommonInfo::AS_Pragma));
+ if (NamedDecl *PrevDecl =
+ LookupSingleName(TUScope, Name, NameLoc, LookupOrdinaryName)) {
+ // Warn if making this declaration weak might invalidate assumptions we've
+ // already made about it.
+ if (const auto *VD = dyn_cast<ValueDecl>(PrevDecl)) {
+ if (VD->isUsed(false) && !VD->isWeak() &&
+ (VD->hasDefiningAttr() || !getDefinition(VD))) {
+ Diag(VD->getLocation(), diag::warn_attribute_weak_after_use) << VD;
+ Diag(W.getLocation(), diag::note_declared_weak_here)
+ << /*IsPragma*/ true << "weak";
+ }
+ }
+
+ // FIXME: We should complain if the declaration has already been defined.
+ DeclApplyPragmaWeak(TUScope, PrevDecl, W);
} else {
(void)WeakUndeclaredIdentifiers.insert(
- std::pair<IdentifierInfo*,WeakInfo>
- (Name, WeakInfo((IdentifierInfo*)nullptr, NameLoc)));
+ std::pair<IdentifierInfo *, WeakInfo>(Name, W));
}
}
-void Sema::ActOnPragmaWeakAlias(IdentifierInfo* Name,
- IdentifierInfo* AliasName,
+void Sema::ActOnPragmaWeakAlias(IdentifierInfo *Name,
+ IdentifierInfo *AliasName,
SourceLocation PragmaLoc,
SourceLocation NameLoc,
SourceLocation AliasNameLoc) {
- Decl *PrevDecl = LookupSingleName(TUScope, AliasName, AliasNameLoc,
- LookupOrdinaryName);
+ NamedDecl *PrevDecl =
+ LookupSingleName(TUScope, AliasName, AliasNameLoc, LookupOrdinaryName);
WeakInfo W = WeakInfo(Name, NameLoc);
if (PrevDecl && (isa<FunctionDecl>(PrevDecl) || isa<VarDecl>(PrevDecl))) {
if (!PrevDecl->hasAttr<AliasAttr>())
- if (NamedDecl *ND = dyn_cast<NamedDecl>(PrevDecl))
- DeclApplyPragmaWeak(TUScope, ND, W);
+ DeclApplyPragmaWeak(TUScope, PrevDecl, W);
} else {
(void)WeakUndeclaredIdentifiers.insert(
- std::pair<IdentifierInfo*,WeakInfo>(AliasName, W));
+ std::pair<IdentifierInfo *, WeakInfo>(AliasName, W));
}
}
Index: clang/lib/AST/DeclBase.cpp
===================================================================
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -619,6 +619,11 @@
return AR_Available;
}
+// Declared in include/clang/Basic/Attr.td.
+AvailabilityResult AvailabilityAttr::getAvailability(ASTContext &C) const {
+ return CheckAvailability(C, this, nullptr, VersionTuple());
+}
+
AvailabilityResult Decl::getAvailability(std::string *Message,
VersionTuple EnclosingVersion,
StringRef *RealizedPlatform) const {
@@ -725,8 +730,7 @@
return true;
if (const auto *Availability = dyn_cast<AvailabilityAttr>(A)) {
- if (CheckAvailability(getASTContext(), Availability, nullptr,
- VersionTuple()) == AR_NotYetIntroduced)
+ if (Availability->getAvailability(getASTContext()) == AR_NotYetIntroduced)
return true;
}
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3247,6 +3247,11 @@
InGroup<DiagGroup<"unsupported-dll-base-class-template">>, DefaultIgnore;
def err_attribute_dll_ambiguous_default_ctor : Error<
"'__declspec(dllexport)' cannot be applied to more than one default constructor in %0">;
+def warn_attribute_weak_after_use : Warning<
+ "%0 declared weak after its first use; weakness may not be respected in all contexts">,
+ InGroup<DiagGroup<"weak-after-use">>;
+def note_declared_weak_here : Note<
+ "declared weak by %select{'%1' attribute|pragma '%1'}0 here">;
def err_attribute_weakref_not_static : Error<
"weakref declaration must have internal linkage">;
def err_attribute_weakref_not_global_context : Error<
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -839,7 +839,10 @@
.Case("tvOSApplicationExtension", "tvos_app_extension")
.Case("watchOSApplicationExtension", "watchos_app_extension")
.Default(Platform);
-} }];
+}
+// Defined in lib/AST/DeclBase.cpp.
+AvailabilityResult getAvailability(ASTContext &C) const;
+}];
let HasCustomParsing = 1;
let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Named]>;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
