Skip to content

Commit 5343b59

Browse files
committed
Don't try to apply fix-its for renamed operators.
...or renaming to operators, unless we're sure the original thing was an operator expression. There are just a lot of ways this can be screwed up. (Some cases of this can certainly be implemented. I may file a StarterBug later.)
1 parent ced32f5 commit 5343b59

File tree

4 files changed

+72
-28
lines changed

4 files changed

+72
-28
lines changed

lib/Sema/MiscDiagnostics.cpp

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,27 +1006,37 @@ void swift::fixItAvailableAttrRename(TypeChecker &TC,
10061006
InFlightDiagnostic &diag,
10071007
SourceRange referenceRange,
10081008
const AvailableAttr *attr,
1009-
const CallExpr *CE) {
1009+
const ApplyExpr *call) {
10101010
ParsedDeclName parsed = swift::parseDeclName(attr->Rename);
10111011
if (!parsed)
10121012
return;
10131013

1014+
bool originallyWasKnownOperatorExpr = false;
1015+
if (call) {
1016+
originallyWasKnownOperatorExpr =
1017+
isa<BinaryExpr>(call) ||
1018+
isa<PrefixUnaryExpr>(call) ||
1019+
isa<PostfixUnaryExpr>(call);
1020+
}
1021+
if (parsed.isOperator() != originallyWasKnownOperatorExpr)
1022+
return;
1023+
10141024
SourceManager &sourceMgr = TC.Context.SourceMgr;
10151025

10161026
if (parsed.isInstanceMember()) {
10171027
// Replace the base of the call with the "self argument".
10181028
// We can only do a good job with the fix-it if we have the whole call
10191029
// expression.
10201030
// FIXME: Should we be validating the ContextName in some way?
1021-
if (!CE)
1031+
if (!dyn_cast_or_null<CallExpr>(call))
10221032
return;
10231033

10241034
unsigned selfIndex = parsed.SelfIndex.getValue();
10251035
const Expr *selfExpr = nullptr;
10261036
SourceLoc removeRangeStart;
10271037
SourceLoc removeRangeEnd;
10281038

1029-
const Expr *argExpr = CE->getArg();
1039+
const Expr *argExpr = call->getArg();
10301040
if (auto args = dyn_cast<TupleExpr>(argExpr)) {
10311041
size_t numElementsWithinParens = args->getNumElements();
10321042
numElementsWithinParens -= args->hasTrailingClosure();
@@ -1115,21 +1125,22 @@ void swift::fixItAvailableAttrRename(TypeChecker &TC,
11151125
selfReplace.push_back(')');
11161126
selfReplace.push_back('.');
11171127
selfReplace += parsed.BaseName;
1118-
diag.fixItReplace(CE->getFn()->getSourceRange(), selfReplace);
1128+
diag.fixItReplace(call->getFn()->getSourceRange(), selfReplace);
11191129

11201130
if (!parsed.isPropertyAccessor())
11211131
diag.fixItRemoveChars(removeRangeStart, removeRangeEnd);
11221132

11231133
// Continue on to diagnose any argument label renames.
11241134

1125-
} else if (parsed.BaseName == TC.Context.Id_init.str() && CE) {
1135+
} else if (parsed.BaseName == TC.Context.Id_init.str() &&
1136+
dyn_cast_or_null<CallExpr>(call)) {
11261137
// For initializers, replace with a "call" of the context type...but only
11271138
// if we know we're doing a call (rather than a first-class reference).
11281139
if (parsed.isMember()) {
1129-
diag.fixItReplace(CE->getFn()->getSourceRange(), parsed.ContextName);
1140+
diag.fixItReplace(call->getFn()->getSourceRange(), parsed.ContextName);
11301141

11311142
} else {
1132-
auto *dotCall = dyn_cast<DotSyntaxCallExpr>(CE->getFn());
1143+
auto *dotCall = dyn_cast<DotSyntaxCallExpr>(call->getFn());
11331144
if (!dotCall)
11341145
return;
11351146

@@ -1151,10 +1162,10 @@ void swift::fixItAvailableAttrRename(TypeChecker &TC,
11511162
diag.fixItReplace(referenceRange, baseReplace);
11521163
}
11531164

1154-
if (!CE)
1165+
if (!dyn_cast_or_null<CallExpr>(call))
11551166
return;
11561167

1157-
const Expr *argExpr = CE->getArg();
1168+
const Expr *argExpr = call->getArg();
11581169
if (parsed.IsGetter) {
11591170
diag.fixItRemove(argExpr->getSourceRange());
11601171
return;
@@ -1297,7 +1308,7 @@ void TypeChecker::diagnoseDeprecated(SourceRange ReferenceRange,
12971308
const DeclContext *ReferenceDC,
12981309
const AvailableAttr *Attr,
12991310
DeclName Name,
1300-
const CallExpr *CE) {
1311+
const ApplyExpr *Call) {
13011312
// We match the behavior of clang to not report deprecation warnings
13021313
// inside declarations that are themselves deprecated on all deployment
13031314
// targets.
@@ -1354,7 +1365,7 @@ void TypeChecker::diagnoseDeprecated(SourceRange ReferenceRange,
13541365
auto renameDiag = diagnose(ReferenceRange.Start,
13551366
diag::note_deprecated_rename,
13561367
newName);
1357-
fixItAvailableAttrRename(*this, renameDiag, ReferenceRange, Attr, CE);
1368+
fixItAvailableAttrRename(*this, renameDiag, ReferenceRange, Attr, Call);
13581369
}
13591370
}
13601371

@@ -1407,11 +1418,11 @@ void TypeChecker::diagnoseUnavailableOverride(ValueDecl *override,
14071418
bool TypeChecker::diagnoseExplicitUnavailability(const ValueDecl *D,
14081419
SourceRange R,
14091420
const DeclContext *DC,
1410-
const CallExpr *CE) {
1421+
const ApplyExpr *call) {
14111422
return diagnoseExplicitUnavailability(D, R, DC,
14121423
[=](InFlightDiagnostic &diag) {
14131424
fixItAvailableAttrRename(*this, diag, R, AvailableAttr::isUnavailable(D),
1414-
CE);
1425+
call);
14151426
});
14161427
}
14171428

@@ -1546,19 +1557,19 @@ class AvailabilityWalker : public ASTWalker {
15461557

15471558
if (auto DR = dyn_cast<DeclRefExpr>(E))
15481559
diagAvailability(DR->getDecl(), DR->getSourceRange(),
1549-
getEnclosingCallExpr());
1560+
getEnclosingApplyExpr());
15501561
if (auto MR = dyn_cast<MemberRefExpr>(E)) {
15511562
walkMemberRef(MR);
15521563
return skipChildren();
15531564
}
15541565
if (auto OCDR = dyn_cast<OtherConstructorDeclRefExpr>(E))
15551566
diagAvailability(OCDR->getDecl(),
15561567
OCDR->getConstructorLoc().getSourceRange(),
1557-
getEnclosingCallExpr());
1568+
getEnclosingApplyExpr());
15581569
if (auto DMR = dyn_cast<DynamicMemberRefExpr>(E))
15591570
diagAvailability(DMR->getMember().getDecl(),
15601571
DMR->getNameLoc().getSourceRange(),
1561-
getEnclosingCallExpr());
1572+
getEnclosingApplyExpr());
15621573
if (auto DS = dyn_cast<DynamicSubscriptExpr>(E))
15631574
diagAvailability(DS->getMember().getDecl(), DS->getSourceRange());
15641575
if (auto S = dyn_cast<SubscriptExpr>(E)) {
@@ -1586,12 +1597,12 @@ class AvailabilityWalker : public ASTWalker {
15861597

15871598
private:
15881599
bool diagAvailability(const ValueDecl *D, SourceRange R,
1589-
const CallExpr *CE = nullptr);
1600+
const ApplyExpr *call = nullptr);
15901601
bool diagnoseIncDecRemoval(const ValueDecl *D, SourceRange R,
15911602
const AvailableAttr *Attr);
15921603

1593-
/// Walks up from a potential callee to the enclosing CallExpr.
1594-
const CallExpr *getEnclosingCallExpr() const {
1604+
/// Walks up from a potential callee to the enclosing ApplyExpr.
1605+
const ApplyExpr *getEnclosingApplyExpr() const {
15951606
ArrayRef<const Expr *> parents = ExprStack;
15961607
assert(!parents.empty() && "must be called while visiting an expression");
15971608
size_t idx = parents.size() - 1;
@@ -1606,7 +1617,7 @@ class AvailabilityWalker : public ASTWalker {
16061617
isa<ForceValueExpr>(parents[idx]) || // f!(a)
16071618
isa<BindOptionalExpr>(parents[idx])); // f?(a)
16081619

1609-
auto *call = dyn_cast<CallExpr>(parents[idx]);
1620+
auto *call = dyn_cast<ApplyExpr>(parents[idx]);
16101621
if (!call || call->getFn() != parents[idx+1])
16111622
return nullptr;
16121623
return call;
@@ -1723,20 +1734,20 @@ class AvailabilityWalker : public ASTWalker {
17231734
/// Diagnose uses of unavailable declarations. Returns true if a diagnostic
17241735
/// was emitted.
17251736
bool AvailabilityWalker::diagAvailability(const ValueDecl *D, SourceRange R,
1726-
const CallExpr *CE) {
1737+
const ApplyExpr *call) {
17271738
if (!D)
17281739
return false;
17291740

17301741
if (auto *attr = AvailableAttr::isUnavailable(D))
17311742
if (diagnoseIncDecRemoval(D, R, attr))
17321743
return true;
17331744

1734-
if (TC.diagnoseExplicitUnavailability(D, R, DC, CE))
1745+
if (TC.diagnoseExplicitUnavailability(D, R, DC, call))
17351746
return true;
17361747

17371748
// Diagnose for deprecation
17381749
if (const AvailableAttr *Attr = TypeChecker::getDeprecated(D)) {
1739-
TC.diagnoseDeprecated(R, DC, Attr, D->getFullName(), CE);
1750+
TC.diagnoseDeprecated(R, DC, Attr, D->getFullName(), call);
17401751
}
17411752

17421753
if (TC.getLangOpts().DisableAvailabilityChecking)

lib/Sema/MiscDiagnostics.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222

2323
namespace swift {
2424
class AbstractFunctionDecl;
25+
class ApplyExpr;
2526
class AvailableAttr;
26-
class CallExpr;
2727
class DeclContext;
2828
class Expr;
2929
class InFlightDiagnostic;
@@ -61,13 +61,13 @@ bool diagnoseArgumentLabelError(TypeChecker &TC, const Expr *expr,
6161
InFlightDiagnostic *existingDiag = nullptr);
6262

6363
/// Emit fix-its to rename the base name at \p referenceRange based on the
64-
/// "renamed" argument in \p attr. If \p CE is provided, the argument labels
64+
/// "renamed" argument in \p attr. If \p call is provided, the argument labels
6565
/// will also be updated.
6666
void fixItAvailableAttrRename(TypeChecker &TC,
6767
InFlightDiagnostic &diag,
6868
SourceRange referenceRange,
6969
const AvailableAttr *attr,
70-
const CallExpr *CE);
70+
const ApplyExpr *call);
7171
} // namespace swift
7272

7373
#endif // SWIFT_SEMA_MISC_DIAGNOSTICS_H

lib/Sema/TypeChecker.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,7 +1608,7 @@ class TypeChecker final : public LazyResolver {
16081608
bool diagnoseExplicitUnavailability(const ValueDecl *D,
16091609
SourceRange R,
16101610
const DeclContext *DC,
1611-
const CallExpr *CE);
1611+
const ApplyExpr *call);
16121612

16131613
/// Emit a diagnostic for references to declarations that have been
16141614
/// marked as unavailable, either through "unavailable" or "obsoleted:".
@@ -1821,7 +1821,7 @@ class TypeChecker final : public LazyResolver {
18211821
const DeclContext *ReferenceDC,
18221822
const AvailableAttr *Attr,
18231823
DeclName Name,
1824-
const CallExpr *CE);
1824+
const ApplyExpr *Call);
18251825
/// @}
18261826

18271827
/// If LangOptions::DebugForbidTypecheckPrefix is set and the given decl

test/attr/attr_availability.swift

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,39 @@ func testInstanceTrailingClosure() {
492492
trailingClosureArg2(0, 1) {} // expected-error {{'trailingClosureArg2(_:_:fn:)' has been replaced by instance method 'Int.foo(bar:execute:)'}} {{3-22=1.foo}} {{23-23=bar: }} {{24-27=}}
493493
}
494494

495+
@available(*, unavailable, renamed: "+")
496+
func add(_ value: Int, _ other: Int) {} // expected-note {{here}}
497+
498+
infix operator *** {}
499+
@available(*, unavailable, renamed: "add")
500+
func ***(value: (), other: ()) {} // expected-note {{here}}
501+
@available(*, unavailable, renamed: "Int.foo(self:_:)")
502+
func ***(value: Int, other: Int) {} // expected-note {{here}}
503+
504+
prefix operator *** {}
505+
@available(*, unavailable, renamed: "add")
506+
prefix func ***(value: Int?) {} // expected-note {{here}}
507+
@available(*, unavailable, renamed: "Int.foo(self:)")
508+
prefix func ***(value: Int) {} // expected-note {{here}}
509+
510+
postfix operator *** {}
511+
@available(*, unavailable, renamed: "add")
512+
postfix func ***(value: Int?) {} // expected-note {{here}}
513+
@available(*, unavailable, renamed: "Int.foo(self:)")
514+
postfix func ***(value: Int) {} // expected-note {{here}}
515+
516+
func testOperators() {
517+
add(0, 1) // expected-error {{'add' has been renamed to '+'}} {{none}}
518+
() *** () // expected-error {{'***' has been renamed to 'add'}} {{none}}
519+
0 *** 1 // expected-error {{'***' has been replaced by instance method 'Int.foo(_:)'}} {{none}}
520+
521+
***nil // expected-error {{'***' has been renamed to 'add'}} {{none}}
522+
***0 // expected-error {{'***' has been replaced by instance method 'Int.foo()'}} {{none}}
523+
524+
nil*** // expected-error {{'***' has been renamed to 'add'}} {{none}}
525+
0*** // expected-error {{'***' has been replaced by instance method 'Int.foo()'}} {{none}}
526+
}
527+
495528
extension Int {
496529
@available(*, unavailable, renamed: "init(other:)")
497530
@discardableResult

0 commit comments

Comments
 (0)