Skip to content

Commit 982a2cb

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 784c4a5 commit 982a2cb

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
@@ -6576,6 +6576,10 @@ class AccessorDecl final : public FuncDecl {
65766576
Bits.AccessorDecl.IsTransparentComputed = 1;
65776577
}
65786578

6579+
/// A representation of the name to be displayed to users. \c getNameStr
6580+
/// for anything other than a getter or setter.
6581+
void printUserFacingName(llvm::raw_ostream &out) const;
6582+
65796583
static bool classof(const Decl *D) {
65806584
return D->getKind() == DeclKind::Accessor;
65816585
}

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
@@ -880,17 +880,10 @@ bool DeclAttribute::printImpl(ASTPrinter &Printer, const PrintOptions &Options,
880880
} else if (Attr->RenameDecl) {
881881
Printer << ", renamed: \"";
882882
if (auto *Accessor = dyn_cast<AccessorDecl>(Attr->RenameDecl)) {
883-
switch (Accessor->getAccessorKind()) {
884-
case AccessorKind::Get:
885-
Printer << "getter:";
886-
break;
887-
case AccessorKind::Set:
888-
Printer << "setter:";
889-
break;
890-
default:
891-
break;
892-
}
893-
Printer << Accessor->getStorage()->getName() << "()";
883+
SmallString<32> Name;
884+
llvm::raw_svector_ostream OS(Name);
885+
Accessor->printUserFacingName(OS);
886+
Printer << Name.str();
894887
} else {
895888
Printer << Attr->RenameDecl->getName();
896889
}

lib/AST/Decl.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7839,6 +7839,28 @@ bool AccessorDecl::isSimpleDidSet() const {
78397839
SimpleDidSetRequest{mutableThis}, false);
78407840
}
78417841

7842+
void AccessorDecl::printUserFacingName(raw_ostream &out) const {
7843+
switch (getAccessorKind()) {
7844+
case AccessorKind::Get:
7845+
out << "getter:";
7846+
break;
7847+
case AccessorKind::Set:
7848+
out << "setter:";
7849+
break;
7850+
default:
7851+
out << getName();
7852+
return;
7853+
}
7854+
7855+
out << getStorage()->getName() << "(";
7856+
if (this->isSetter()) {
7857+
for (const auto *param : *getParameters()) {
7858+
out << param->getName() << ":";
7859+
}
7860+
}
7861+
out << ")";
7862+
}
7863+
78427864
StaticSpellingKind FuncDecl::getCorrectStaticSpelling() const {
78437865
assert(getDeclContext()->isTypeContext());
78447866
if (!isStatic())

lib/Sema/MiscDiagnostics.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4673,8 +4673,17 @@ class CompletionHandlerUsageChecker final : public ASTWalker {
46734673
if (!asyncFunc)
46744674
return {false, call};
46754675
ctx.Diags.diagnose(call->getLoc(), diag::warn_use_async_alternative);
4676-
ctx.Diags.diagnose(asyncFunc->getLoc(), diag::decl_declared_here,
4677-
asyncFunc->getName());
4676+
4677+
if (auto *accessor = dyn_cast<AccessorDecl>(asyncFunc)) {
4678+
SmallString<32> name;
4679+
llvm::raw_svector_ostream os(name);
4680+
accessor->printUserFacingName(os);
4681+
ctx.Diags.diagnose(asyncFunc->getLoc(),
4682+
diag::descriptive_decl_declared_here, name);
4683+
} else {
4684+
ctx.Diags.diagnose(asyncFunc->getLoc(), diag::decl_declared_here,
4685+
asyncFunc->getName());
4686+
}
46784687
}
46794688
}
46804689
}

lib/Sema/TypeCheckAttr.cpp

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

5694-
// Kinds have to match
5695-
if (candidate->getKind() != original->getKind())
5694+
// Kinds have to match, but we want to allow eg. an accessor to match
5695+
// a function
5696+
if (candidate->getKind() != original->getKind() &&
5697+
!(isa<FuncDecl>(candidate) && isa<FuncDecl>(original)))
56965698
return false;
56975699

56985700
// Instance can't match static/class function
@@ -5766,12 +5768,24 @@ ValueDecl *RenamedDeclRequest::evaluate(Evaluator &evaluator,
57665768
objc_translation::isVisibleToObjC(attached, minAccess);
57675769

57685770
SmallVector<ValueDecl *, 4> lookupResults;
5771+
SmallVector<AbstractFunctionDecl *, 4> asyncResults;
57695772
lookupReplacedDecl(nameRef, attr, attached, lookupResults);
57705773

57715774
ValueDecl *renamedDecl = nullptr;
57725775
auto attachedFunc = dyn_cast<AbstractFunctionDecl>(attached);
5773-
bool candidateHasAsync = false;
57745776
for (auto candidate : lookupResults) {
5777+
// If the name is a getter or setter, grab the underlying accessor (if any)
5778+
if (parsedName.IsGetter || parsedName.IsSetter) {
5779+
auto *VD = dyn_cast<VarDecl>(candidate);
5780+
if (!VD)
5781+
continue;
5782+
5783+
candidate = VD->getAccessor(parsedName.IsGetter ? AccessorKind::Get :
5784+
AccessorKind::Set);
5785+
if (!candidate)
5786+
continue;
5787+
}
5788+
57755789
if (!renameCouldMatch(attached, candidate, attachedIsObjcVisible,
57765790
minAccess))
57775791
continue;
@@ -5780,7 +5794,8 @@ ValueDecl *RenamedDeclRequest::evaluate(Evaluator &evaluator,
57805794
// Require both functions to be async/not. Async alternatives are handled
57815795
// below if there's no other matches
57825796
if (attachedFunc->hasAsync() != candidateFunc->hasAsync()) {
5783-
candidateHasAsync |= candidateFunc->hasAsync();
5797+
if (candidateFunc->hasAsync())
5798+
asyncResults.push_back(candidateFunc);
57845799
continue;
57855800
}
57865801

@@ -5801,18 +5816,10 @@ ValueDecl *RenamedDeclRequest::evaluate(Evaluator &evaluator,
58015816

58025817
// Try to match up an async alternative instead (ie. one where the
58035818
// completion handler has been removed).
5804-
if (!renamedDecl && candidateHasAsync) {
5805-
for (ValueDecl *candidate : lookupResults) {
5806-
auto *candidateFunc = dyn_cast<AbstractFunctionDecl>(candidate);
5807-
if (!candidateFunc || !candidateFunc->hasAsync())
5808-
continue;
5809-
5810-
if (!renameCouldMatch(attached, candidate, attachedIsObjcVisible,
5811-
minAccess))
5812-
continue;
5813-
5819+
if (!renamedDecl && !asyncResults.empty()) {
5820+
for (AbstractFunctionDecl *candidate : asyncResults) {
58145821
Optional<unsigned> completionHandler =
5815-
attachedFunc->findPotentialCompletionHandlerParam(candidateFunc);
5822+
attachedFunc->findPotentialCompletionHandlerParam(candidate);
58165823
if (!completionHandler)
58175824
continue;
58185825

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)