Skip to content

Commit 17d8c82

Browse files
slavapestovslavapestov
authored
Merge pull request #79220 from slavapestov/fix-rdar143950572
AST: Source range of FuncDecl/ConstructorDecl should include the thrown type
2 parents 87081c0 + e9266c2 commit 17d8c82

File tree

11 files changed

+89
-52
lines changed

11 files changed

+89
-52
lines changed

‎include/swift/AST/Decl.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8081,7 +8081,11 @@ class AbstractFunctionDecl : public GenericContext, public ValueDecl {
80818081
/// source buffers for e.g. code-completion.
80828082
SourceRange getOriginalBodySourceRange() const;
80838083

8084-
/// Retrieve the source range of the function declaration name + patterns.
8084+
/// Retrieve the source range of the function declaration name and parameter list.
8085+
SourceRange getParameterListSourceRange() const;
8086+
8087+
/// Retrieve the source range of the function declaration name, parameter list,
8088+
/// and effects. For FuncDecl, this does not include the return type.
80858089
SourceRange getSignatureSourceRange() const;
80868090

80878091
CaptureInfo getCaptureInfo() const;

‎lib/AST/Decl.cpp

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9939,13 +9939,30 @@ SourceRange AbstractFunctionDecl::getSignatureSourceRange() const {
99399939
if (isImplicit())
99409940
return SourceRange();
99419941

9942-
auto paramList = getParameters();
9942+
SourceLoc endLoc;
99439943

9944-
auto endLoc = paramList->getSourceRange().End;
9945-
if (endLoc.isValid())
9946-
return SourceRange(getNameLoc(), endLoc);
9944+
// name(parameter list...) async throws(E)
9945+
if (auto *typeRepr = getThrownTypeRepr())
9946+
endLoc = typeRepr->getSourceRange().End;
9947+
if (endLoc.isInvalid())
9948+
endLoc = getThrowsLoc();
9949+
if (endLoc.isInvalid())
9950+
endLoc = getAsyncLoc();
99479951

9948-
return getNameLoc();
9952+
if (endLoc.isInvalid())
9953+
return getParameterListSourceRange();
9954+
return SourceRange(getNameLoc(), endLoc);
9955+
}
9956+
9957+
SourceRange AbstractFunctionDecl::getParameterListSourceRange() const {
9958+
if (isImplicit())
9959+
return SourceRange();
9960+
9961+
auto endLoc = getParameters()->getSourceRange().End;
9962+
if (endLoc.isInvalid())
9963+
return getNameLoc();
9964+
9965+
return SourceRange(getNameLoc(), endLoc);
99499966
}
99509967

99519968
std::optional<Fingerprint> AbstractFunctionDecl::getBodyFingerprint() const {
@@ -10982,43 +10999,32 @@ DestructorDecl *DestructorDecl::getSuperDeinit() const {
1098210999
}
1098311000

1098411001
SourceRange FuncDecl::getSourceRange() const {
10985-
SourceLoc StartLoc = getStartLoc();
11002+
SourceLoc startLoc = getStartLoc();
1098611003

10987-
if (StartLoc.isInvalid())
11004+
if (startLoc.isInvalid())
1098811005
return SourceRange();
1098911006

1099011007
if (getBodyKind() == BodyKind::Unparsed)
10991-
return { StartLoc, BodyRange.End };
10992-
10993-
SourceLoc RBraceLoc = getOriginalBodySourceRange().End;
10994-
if (RBraceLoc.isValid()) {
10995-
return { StartLoc, RBraceLoc };
10996-
}
10997-
10998-
if (isa<AccessorDecl>(this))
10999-
return StartLoc;
11000-
11001-
if (getBodyKind() == BodyKind::Synthesize)
11002-
return SourceRange();
11003-
11004-
auto TrailingWhereClauseSourceRange = getGenericTrailingWhereClauseSourceRange();
11005-
if (TrailingWhereClauseSourceRange.isValid())
11006-
return { StartLoc, TrailingWhereClauseSourceRange.End };
11008+
return { startLoc, BodyRange.End };
1100711009

11008-
const auto ResultTyEndLoc = getResultTypeSourceRange().End;
11009-
if (ResultTyEndLoc.isValid())
11010-
return { StartLoc, ResultTyEndLoc };
11010+
SourceLoc endLoc = getOriginalBodySourceRange().End;
11011+
if (endLoc.isInvalid()) {
11012+
if (isa<AccessorDecl>(this))
11013+
return startLoc;
1101111014

11012-
if (hasThrows())
11013-
return { StartLoc, getThrowsLoc() };
11015+
if (getBodyKind() == BodyKind::Synthesize)
11016+
return SourceRange();
1101411017

11015-
if (hasAsync())
11016-
return { StartLoc, getAsyncLoc() };
11018+
endLoc = getGenericTrailingWhereClauseSourceRange().End;
11019+
}
11020+
if (endLoc.isInvalid())
11021+
endLoc = getResultTypeSourceRange().End;
11022+
if (endLoc.isInvalid())
11023+
endLoc = getSignatureSourceRange().End;
11024+
if (endLoc.isInvalid())
11025+
endLoc = startLoc;
1101711026

11018-
auto LastParamListEndLoc = getParameters()->getSourceRange().End;
11019-
if (LastParamListEndLoc.isValid())
11020-
return { StartLoc, LastParamListEndLoc };
11021-
return StartLoc;
11027+
return { startLoc, endLoc };
1102211028
}
1102311029

1102411030
EnumElementDecl::EnumElementDecl(SourceLoc IdentifierLoc, DeclName Name,
@@ -11122,8 +11128,6 @@ SourceRange ConstructorDecl::getSourceRange() const {
1112211128
SourceLoc End = getOriginalBodySourceRange().End;
1112311129
if (End.isInvalid())
1112411130
End = getGenericTrailingWhereClauseSourceRange().End;
11125-
if (End.isInvalid())
11126-
End = getThrowsLoc();
1112711131
if (End.isInvalid())
1112811132
End = getSignatureSourceRange().End;
1112911133

‎lib/IDE/SyntaxModel.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -873,7 +873,7 @@ ASTWalker::PreWalkAction ModelASTWalker::walkToDeclPre(Decl *D) {
873873
SN.BodyRange = innerCharSourceRangeFromSourceRange(SM,
874874
AFD->getBodySourceRange());
875875
SN.NameRange = charSourceRangeFromSourceRange(SM,
876-
AFD->getSignatureSourceRange());
876+
AFD->getParameterListSourceRange());
877877
if (FD) {
878878
SN.TypeRange = charSourceRangeFromSourceRange(SM,
879879
FD->getResultTypeSourceRange());

‎lib/Sema/CSApply.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8409,7 +8409,6 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
84098409
// If this is a call to a distributed method thunk,
84108410
// let's mark the call as implicitly throwing.
84118411
if (isDistributedThunk(callee, apply->getFn())) {
8412-
auto *FD = cast<AbstractFunctionDecl>(callee.getDecl());
84138412
apply->setImplicitlyThrows(true);
84148413
}
84158414

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2545,7 +2545,7 @@ InterfaceTypeRequest::evaluate(Evaluator &eval, ValueDecl *D) const {
25452545
if (thrownTy) {
25462546
thrownTy = AFD->getThrownInterfaceType();
25472547
ProtocolDecl *errorProto = Context.getErrorDecl();
2548-
if (thrownTy && errorProto) {
2548+
if (thrownTy && !thrownTy->hasError() && errorProto) {
25492549
Type thrownTyInContext = AFD->mapTypeIntoContext(thrownTy);
25502550
if (!checkConformance(thrownTyInContext, errorProto)) {
25512551
SourceLoc loc;

‎lib/Sema/TypeCheckType.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4278,12 +4278,15 @@ NeverNullType TypeResolver::resolveASTFunctionType(
42784278
thrownTy = resolveType(thrownTypeRepr, thrownTypeOptions);
42794279
if (thrownTy->hasError()) {
42804280
thrownTy = Type();
4281-
} else if (!options.contains(TypeResolutionFlags::SilenceErrors) &&
4282-
!thrownTy->hasTypeParameter() &&
4283-
!checkConformance(thrownTy, ctx.getErrorDecl())) {
4284-
diagnoseInvalid(
4285-
thrownTypeRepr, thrownTypeRepr->getLoc(), diag::thrown_type_not_error,
4286-
thrownTy);
4281+
} else if (inStage(TypeResolutionStage::Interface) &&
4282+
!options.contains(TypeResolutionFlags::SilenceErrors)) {
4283+
auto thrownTyInContext = GenericEnvironment::mapTypeIntoContext(
4284+
resolution.getGenericSignature().getGenericEnvironment(), thrownTy);
4285+
if (!checkConformance(thrownTyInContext, ctx.getErrorDecl())) {
4286+
diagnoseInvalid(
4287+
thrownTypeRepr, thrownTypeRepr->getLoc(), diag::thrown_type_not_error,
4288+
thrownTy);
4289+
}
42874290
}
42884291
}
42894292

‎test/Generics/typed_throws.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ func f2<T: P1>(_: T) where () throws(T.A) -> () == () throws -> () {}
1414

1515
protocol P2 {
1616
associatedtype A where A == () throws(E) -> ()
17-
associatedtype E
17+
associatedtype E: Error
1818
}
1919

2020
// CHECK-LABEL: typed_throws.(file).f3@

‎test/IDE/structure.swift

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,18 @@ struct MyStruct {
4242
static func cfoo()
4343
}
4444

45+
enum MyError: Error {
46+
case x
47+
}
48+
4549
// CHECK: <protocol>protocol <name>MyProt</name> {
4650
// CHECK: <ifunc>func <name>foo()</name></ifunc>
4751
// CHECK: <ifunc>func <name>foo2()</name> throws</ifunc>
4852
// CHECK: <ifunc>func <name>foo3()</name> throws -> <type>Int</type></ifunc>
49-
// CHECK: <ifunc>func <name>foo4<<generic-param><name>T</name></generic-param>>()</name> where T: MyProt</ifunc>
53+
54+
// FIXME: The end of the source range needs to be advanced past the ')' here!
55+
// CHECK: <ifunc>func <name>foo4()</name> throws(MyError</ifunc>)
56+
// CHECK: <ifunc>func <name>foo5<<generic-param><name>T</name></generic-param>>()</name> where T: MyProt</ifunc>
5057
// CHECK: <ifunc><name>init()</name></ifunc>
5158
// CHECK: <ifunc><name>init(<param><name>a</name>: <type>Int</type></param>)</name> throws</ifunc>
5259
// CHECK: <ifunc><name>init<<generic-param><name>T</name></generic-param>>(<param><name>a</name>: <type>T</type></param>)</name> where T: MyProt</ifunc>
@@ -55,7 +62,8 @@ protocol MyProt {
5562
func foo()
5663
func foo2() throws
5764
func foo3() throws -> Int
58-
func foo4<T>() where T: MyProt
65+
func foo4() throws(MyError)
66+
func foo5<T>() where T: MyProt
5967
init()
6068
init(a: Int) throws
6169
init<T>(a: T) where T: MyProt

‎test/decl/func/typed_throws.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
// RUN: %target-typecheck-verify-swift -swift-version 5 -module-name test
22

3+
// Don't complain about <<error type>> not conforming to Error
4+
func invalidThrownType() throws(DoesNotExist) {}
5+
// expected-error@-1 {{cannot find type 'DoesNotExist' in scope}}
6+
37
// expected-note@+1{{type declared here}}
48
enum MyError: Error {
59
case fail
@@ -207,3 +211,8 @@ extension Result {
207211
}
208212
}
209213
}
214+
215+
struct NotAnError<T> {}
216+
217+
func badThrowingFunctionType<T>(_: () throws(NotAnError<T>) -> ()) {}
218+
// expected-error@-1 {{thrown type 'NotAnError<T>' does not conform to the 'Error' protocol}}

‎test/decl/protocol/typed_throws.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
// Make sure that the source range of a protocol requirement
4+
// with no body still includes the thrown type, by checking
5+
// that the name lookup from there finds the generic parameter.
6+
7+
protocol TypedThrowsProto {
8+
init<E>(y: () throws(E) -> Void) throws(E)
9+
func f<E>(y: () throws(E) -> Void) throws(E)
10+
}

‎tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -890,17 +890,17 @@ static void setLocationInfo(const ValueDecl *VD,
890890

891891
auto Loc = VD->getLoc(/*SerializedOK=*/true);
892892
if (Loc.isValid()) {
893-
auto getSignatureRange =
893+
auto getParameterListRange =
894894
[&](const ValueDecl *VD) -> std::optional<unsigned> {
895895
if (auto FD = dyn_cast<AbstractFunctionDecl>(VD)) {
896-
SourceRange R = FD->getSignatureSourceRange();
896+
SourceRange R = FD->getParameterListSourceRange();
897897
if (R.isValid())
898898
return getCharLength(SM, R);
899899
}
900900
return std::nullopt;
901901
};
902902
unsigned NameLen;
903-
if (auto SigLen = getSignatureRange(VD)) {
903+
if (auto SigLen = getParameterListRange(VD)) {
904904
NameLen = SigLen.value();
905905
} else if (VD->hasName()) {
906906
NameLen = VD->getBaseName().userFacingName().size();

0 commit comments

Comments
 (0)