Skip to content

Commit 7e7b521

Browse files
committed
[Diagnostics] Allow matching accessors and functions for a renamed decl
A lookup on the name provided by `renamed` in `@available` returns the VarDecl. If the name specified a getter or setter, make sure to grab the corresponding accessor when comparing candidates. We currently ignore the basename and parameters specified in the name for accessors. Checking them would only cause a getter/setter not to match, there can never be a conflict. Since this is a best effort match anyway, this seems fine.
1 parent aeb36ab commit 7e7b521

File tree

7 files changed

+108
-28
lines changed

7 files changed

+108
-28
lines changed

include/swift/AST/Decl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6599,6 +6599,10 @@ class AccessorDecl final : public FuncDecl {
65996599
Bits.AccessorDecl.IsTransparentComputed = 1;
66006600
}
66016601

6602+
/// A representation of the name to be displayed to users. \c getNameStr
6603+
/// for anything other than a getter or setter.
6604+
void printUserFacingName(llvm::raw_ostream &out) const;
6605+
66026606
static bool classof(const Decl *D) {
66036607
return D->getKind() == DeclKind::Accessor;
66046608
}

include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ NOTE(decl_declared_here,none,
3030
"%0 declared here", (DeclName))
3131
NOTE(kind_declared_here,none,
3232
"%0 declared here", (DescriptiveDeclKind))
33+
NOTE(descriptive_decl_declared_here,none,
34+
"'%0' declared here", (StringRef))
3335
NOTE(implicit_member_declared_here,none,
3436
"%1 '%0' is implicitly declared", (StringRef, StringRef))
3537
NOTE(extended_type_declared_here,none,

lib/AST/Attr.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -886,17 +886,10 @@ bool DeclAttribute::printImpl(ASTPrinter &Printer, const PrintOptions &Options,
886886
} else if (Attr->RenameDecl) {
887887
Printer << ", renamed: \"";
888888
if (auto *Accessor = dyn_cast<AccessorDecl>(Attr->RenameDecl)) {
889-
switch (Accessor->getAccessorKind()) {
890-
case AccessorKind::Get:
891-
Printer << "getter:";
892-
break;
893-
case AccessorKind::Set:
894-
Printer << "setter:";
895-
break;
896-
default:
897-
break;
898-
}
899-
Printer << Accessor->getStorage()->getName() << "()";
889+
SmallString<32> Name;
890+
llvm::raw_svector_ostream OS(Name);
891+
Accessor->printUserFacingName(OS);
892+
Printer << Name.str();
900893
} else {
901894
Printer << Attr->RenameDecl->getName();
902895
}

lib/AST/Decl.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7910,6 +7910,28 @@ bool AccessorDecl::isSimpleDidSet() const {
79107910
SimpleDidSetRequest{mutableThis}, false);
79117911
}
79127912

7913+
void AccessorDecl::printUserFacingName(raw_ostream &out) const {
7914+
switch (getAccessorKind()) {
7915+
case AccessorKind::Get:
7916+
out << "getter:";
7917+
break;
7918+
case AccessorKind::Set:
7919+
out << "setter:";
7920+
break;
7921+
default:
7922+
out << getName();
7923+
return;
7924+
}
7925+
7926+
out << getStorage()->getName() << "(";
7927+
if (this->isSetter()) {
7928+
for (const auto *param : *getParameters()) {
7929+
out << param->getName() << ":";
7930+
}
7931+
}
7932+
out << ")";
7933+
}
7934+
79137935
StaticSpellingKind FuncDecl::getCorrectStaticSpelling() const {
79147936
assert(getDeclContext()->isTypeContext());
79157937
if (!isStatic())

lib/Sema/MiscDiagnostics.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4702,8 +4702,17 @@ class CompletionHandlerUsageChecker final : public ASTWalker {
47024702
if (!asyncFunc)
47034703
return {false, call};
47044704
ctx.Diags.diagnose(call->getLoc(), diag::warn_use_async_alternative);
4705-
ctx.Diags.diagnose(asyncFunc->getLoc(), diag::decl_declared_here,
4706-
asyncFunc->getName());
4705+
4706+
if (auto *accessor = dyn_cast<AccessorDecl>(asyncFunc)) {
4707+
SmallString<32> name;
4708+
llvm::raw_svector_ostream os(name);
4709+
accessor->printUserFacingName(os);
4710+
ctx.Diags.diagnose(asyncFunc->getLoc(),
4711+
diag::descriptive_decl_declared_here, name);
4712+
} else {
4713+
ctx.Diags.diagnose(asyncFunc->getLoc(), diag::decl_declared_here,
4714+
asyncFunc->getName());
4715+
}
47074716
}
47084717
}
47094718
}

lib/Sema/TypeCheckAttr.cpp

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5686,8 +5686,10 @@ static bool renameCouldMatch(const ValueDecl *original,
56865686
if (original == candidate)
56875687
return false;
56885688

5689-
// Kinds have to match
5690-
if (candidate->getKind() != original->getKind())
5689+
// Kinds have to match, but we want to allow eg. an accessor to match
5690+
// a function
5691+
if (candidate->getKind() != original->getKind() &&
5692+
!(isa<FuncDecl>(candidate) && isa<FuncDecl>(original)))
56915693
return false;
56925694

56935695
// Instance can't match static/class function
@@ -5761,12 +5763,24 @@ ValueDecl *RenamedDeclRequest::evaluate(Evaluator &evaluator,
57615763
objc_translation::isVisibleToObjC(attached, minAccess);
57625764

57635765
SmallVector<ValueDecl *, 4> lookupResults;
5766+
SmallVector<AbstractFunctionDecl *, 4> asyncResults;
57645767
lookupReplacedDecl(nameRef, attr, attached, lookupResults);
57655768

57665769
ValueDecl *renamedDecl = nullptr;
57675770
auto attachedFunc = dyn_cast<AbstractFunctionDecl>(attached);
5768-
bool candidateHasAsync = false;
57695771
for (auto candidate : lookupResults) {
5772+
// If the name is a getter or setter, grab the underlying accessor (if any)
5773+
if (parsedName.IsGetter || parsedName.IsSetter) {
5774+
auto *VD = dyn_cast<VarDecl>(candidate);
5775+
if (!VD)
5776+
continue;
5777+
5778+
candidate = VD->getAccessor(parsedName.IsGetter ? AccessorKind::Get :
5779+
AccessorKind::Set);
5780+
if (!candidate)
5781+
continue;
5782+
}
5783+
57705784
if (!renameCouldMatch(attached, candidate, attachedIsObjcVisible,
57715785
minAccess))
57725786
continue;
@@ -5775,7 +5789,8 @@ ValueDecl *RenamedDeclRequest::evaluate(Evaluator &evaluator,
57755789
// Require both functions to be async/not. Async alternatives are handled
57765790
// below if there's no other matches
57775791
if (attachedFunc->hasAsync() != candidateFunc->hasAsync()) {
5778-
candidateHasAsync |= candidateFunc->hasAsync();
5792+
if (candidateFunc->hasAsync())
5793+
asyncResults.push_back(candidateFunc);
57795794
continue;
57805795
}
57815796

@@ -5796,18 +5811,10 @@ ValueDecl *RenamedDeclRequest::evaluate(Evaluator &evaluator,
57965811

57975812
// Try to match up an async alternative instead (ie. one where the
57985813
// completion handler has been removed).
5799-
if (!renamedDecl && candidateHasAsync) {
5800-
for (ValueDecl *candidate : lookupResults) {
5801-
auto *candidateFunc = dyn_cast<AbstractFunctionDecl>(candidate);
5802-
if (!candidateFunc || !candidateFunc->hasAsync())
5803-
continue;
5804-
5805-
if (!renameCouldMatch(attached, candidate, attachedIsObjcVisible,
5806-
minAccess))
5807-
continue;
5808-
5814+
if (!renamedDecl && !asyncResults.empty()) {
5815+
for (AbstractFunctionDecl *candidate : asyncResults) {
58095816
Optional<unsigned> completionHandler =
5810-
attachedFunc->findPotentialCompletionHandlerParam(candidateFunc);
5817+
attachedFunc->findPotentialCompletionHandlerParam(candidate);
58115818
if (!completionHandler)
58125819
continue;
58135820

test/attr/attr_availability_async_rename.swift

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ func platformOnly(completionHandler: @escaping () -> Void) { }
8585
// expected-note@+1 {{'platformOnlyNew()' declared here}}
8686
func platformOnlyNew() async { }
8787

88+
struct AnotherStruct {
89+
var otherInstanceProp: Int { get async { 1 } }
90+
}
91+
8892
struct SomeStruct {
8993
@available(*, renamed: "structFunc")
9094
func structFunc(continuation: @escaping () -> Void) { }
@@ -97,6 +101,31 @@ struct SomeStruct {
97101

98102
// expected-note@+1 2 {{'staticStructFunc()' declared here}}
99103
static func staticStructFunc() async { }
104+
105+
// expected-note@+1 3 {{'getter:instanceProp()' declared here}}
106+
var instanceProp: Int { get async { 1 } }
107+
var regInstanceProp: Int { get { 1 } set { } }
108+
// expected-note@+1 {{'getter:classProp()' declared here}}
109+
static var classProp: Int { get async { 1 } }
110+
111+
@available(*, renamed: "getter:instanceProp()")
112+
func instanceGetter(completion: @escaping (Int) -> Void) { }
113+
@available(*, renamed: "getter:classProp()")
114+
static func classGetter(completion: @escaping (Int) -> Void) { }
115+
@available(*, renamed: "getter:instanceProp(a:b:)")
116+
func argsIgnored(completion: @escaping (Int) -> Void) { }
117+
@available(*, renamed: "getter:DoesNotExist.instanceProp()")
118+
func baseIgnored(completion: @escaping (Int) -> Void) { }
119+
120+
@available(*, renamed: "instanceProp()")
121+
func noPrefix(completion: @escaping (Int) -> Void) { }
122+
@available(*, renamed: "getter:instanceProp()")
123+
func argMismatch(arg: Int, completion: @escaping (Int) -> Void) { }
124+
@available(*, renamed: "setter:regInstanceProp(newValue:)")
125+
func instanceSetter(arg: Int, completion: @escaping (Int) -> Void) { }
126+
127+
@available(*, renamed: "getter:AnotherStruct.otherInstanceProp()")
128+
func otherInstance(completion: @escaping (Int) -> Void) { }
100129
}
101130

102131

@@ -328,6 +357,20 @@ class ClassCallingAsyncStuff {
328357

329358
// expected-warning@+1{{consider using asynchronous alternative function}}
330359
type(of: other).staticStructFunc { }
360+
361+
// expected-warning@+1{{consider using asynchronous alternative function}}
362+
other.instanceGetter { _ in }
363+
// expected-warning@+1{{consider using asynchronous alternative function}}
364+
other.argsIgnored { _ in }
365+
// expected-warning@+1{{consider using asynchronous alternative function}}
366+
other.baseIgnored { _ in }
367+
// expected-warning@+1{{consider using asynchronous alternative function}}
368+
SomeStruct.classGetter { _ in }
369+
370+
other.noPrefix { _ in }
371+
other.argMismatch(arg: 1) { _ in }
372+
other.instanceSetter(arg: 1) { _ in }
373+
other.otherInstance { _ in }
331374
}
332375

333376
// no warning

0 commit comments

Comments
 (0)