Skip to content

Commit 8f4d272

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 598eef3 commit 8f4d272

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
@@ -6589,6 +6589,10 @@ class AccessorDecl final : public FuncDecl {
65896589
Bits.AccessorDecl.IsTransparentComputed = 1;
65906590
}
65916591

6592+
/// A representation of the name to be displayed to users. \c getNameStr
6593+
/// for anything other than a getter or setter.
6594+
void printUserFacingName(llvm::raw_ostream &out) const;
6595+
65926596
static bool classof(const Decl *D) {
65936597
return D->getKind() == DeclKind::Accessor;
65946598
}

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
@@ -7882,6 +7882,28 @@ bool AccessorDecl::isSimpleDidSet() const {
78827882
SimpleDidSetRequest{mutableThis}, false);
78837883
}
78847884

7885+
void AccessorDecl::printUserFacingName(raw_ostream &out) const {
7886+
switch (getAccessorKind()) {
7887+
case AccessorKind::Get:
7888+
out << "getter:";
7889+
break;
7890+
case AccessorKind::Set:
7891+
out << "setter:";
7892+
break;
7893+
default:
7894+
out << getName();
7895+
return;
7896+
}
7897+
7898+
out << getStorage()->getName() << "(";
7899+
if (this->isSetter()) {
7900+
for (const auto *param : *getParameters()) {
7901+
out << param->getName() << ":";
7902+
}
7903+
}
7904+
out << ")";
7905+
}
7906+
78857907
StaticSpellingKind FuncDecl::getCorrectStaticSpelling() const {
78867908
assert(getDeclContext()->isTypeContext());
78877909
if (!isStatic())

lib/Sema/MiscDiagnostics.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4695,8 +4695,17 @@ class CompletionHandlerUsageChecker final : public ASTWalker {
46954695
if (!asyncFunc)
46964696
return {false, call};
46974697
ctx.Diags.diagnose(call->getLoc(), diag::warn_use_async_alternative);
4698-
ctx.Diags.diagnose(asyncFunc->getLoc(), diag::decl_declared_here,
4699-
asyncFunc->getName());
4698+
4699+
if (auto *accessor = dyn_cast<AccessorDecl>(asyncFunc)) {
4700+
SmallString<32> name;
4701+
llvm::raw_svector_ostream os(name);
4702+
accessor->printUserFacingName(os);
4703+
ctx.Diags.diagnose(asyncFunc->getLoc(),
4704+
diag::descriptive_decl_declared_here, name);
4705+
} else {
4706+
ctx.Diags.diagnose(asyncFunc->getLoc(), diag::decl_declared_here,
4707+
asyncFunc->getName());
4708+
}
47004709
}
47014710
}
47024711
}

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)