Merge pull request #79220 from slavapestov/fix-rdar143950572

slavapestov · web-flow · commit 17d8c820dc7c · 2025-02-10T13:49:37.000-05:00
AST: Source range of FuncDecl/ConstructorDecl should include the thrown type
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -8081,7 +8081,11 @@ class AbstractFunctionDecl : public GenericContext, public ValueDecl {
   /// source buffers for e.g. code-completion.
   SourceRange getOriginalBodySourceRange() const;
 
-  /// Retrieve the source range of the function declaration name + patterns.
+  /// Retrieve the source range of the function declaration name and parameter list.
+  SourceRange getParameterListSourceRange() const;
+
+  /// Retrieve the source range of the function declaration name, parameter list,
+  /// and effects. For FuncDecl, this does not include the return type.
   SourceRange getSignatureSourceRange() const;
 
   CaptureInfo getCaptureInfo() const;
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -9939,13 +9939,30 @@ SourceRange AbstractFunctionDecl::getSignatureSourceRange() const {
   if (isImplicit())
     return SourceRange();
 
-  auto paramList = getParameters();
+  SourceLoc endLoc;
 
-  auto endLoc = paramList->getSourceRange().End;
-  if (endLoc.isValid())
-    return SourceRange(getNameLoc(), endLoc);
+  // name(parameter list...) async throws(E)
+  if (auto *typeRepr = getThrownTypeRepr())
+    endLoc = typeRepr->getSourceRange().End;
+  if (endLoc.isInvalid())
+    endLoc = getThrowsLoc();
+  if (endLoc.isInvalid())
+    endLoc = getAsyncLoc();
 
-  return getNameLoc();
+  if (endLoc.isInvalid())
+    return getParameterListSourceRange();
+  return SourceRange(getNameLoc(), endLoc);
+}
+
+SourceRange AbstractFunctionDecl::getParameterListSourceRange() const {
+  if (isImplicit())
+    return SourceRange();
+
+  auto endLoc = getParameters()->getSourceRange().End;
+  if (endLoc.isInvalid())
+    return getNameLoc();
+
+  return SourceRange(getNameLoc(), endLoc);
 }
 
 std::optional<Fingerprint> AbstractFunctionDecl::getBodyFingerprint() const {
@@ -10982,43 +10999,32 @@ DestructorDecl *DestructorDecl::getSuperDeinit() const {
 }
 
 SourceRange FuncDecl::getSourceRange() const {
-  SourceLoc StartLoc = getStartLoc();
+  SourceLoc startLoc = getStartLoc();
 
-  if (StartLoc.isInvalid())
+  if (startLoc.isInvalid())
     return SourceRange();
 
   if (getBodyKind() == BodyKind::Unparsed)
-    return { StartLoc, BodyRange.End };
-
-  SourceLoc RBraceLoc = getOriginalBodySourceRange().End;
-  if (RBraceLoc.isValid()) {
-    return { StartLoc, RBraceLoc };
-  }
-
-  if (isa<AccessorDecl>(this))
-    return StartLoc;
-
-  if (getBodyKind() == BodyKind::Synthesize)
-    return SourceRange();
-
-  auto TrailingWhereClauseSourceRange = getGenericTrailingWhereClauseSourceRange();
-  if (TrailingWhereClauseSourceRange.isValid())
-    return { StartLoc, TrailingWhereClauseSourceRange.End };
+    return { startLoc, BodyRange.End };
 
-  const auto ResultTyEndLoc = getResultTypeSourceRange().End;
-  if (ResultTyEndLoc.isValid())
-    return { StartLoc, ResultTyEndLoc };
+  SourceLoc endLoc = getOriginalBodySourceRange().End;
+  if (endLoc.isInvalid()) {
+    if (isa<AccessorDecl>(this))
+      return startLoc;
 
-  if (hasThrows())
-    return { StartLoc, getThrowsLoc() };
+    if (getBodyKind() == BodyKind::Synthesize)
+      return SourceRange();
 
-  if (hasAsync())
-    return { StartLoc, getAsyncLoc() };
+    endLoc = getGenericTrailingWhereClauseSourceRange().End;
+  }
+  if (endLoc.isInvalid())
+    endLoc = getResultTypeSourceRange().End;
+  if (endLoc.isInvalid())
+    endLoc = getSignatureSourceRange().End;
+  if (endLoc.isInvalid())
+    endLoc = startLoc;
 
-  auto LastParamListEndLoc = getParameters()->getSourceRange().End;
-  if (LastParamListEndLoc.isValid())
-    return { StartLoc, LastParamListEndLoc };
-  return StartLoc;
+  return { startLoc, endLoc };
 }
 
 EnumElementDecl::EnumElementDecl(SourceLoc IdentifierLoc, DeclName Name,
@@ -11122,8 +11128,6 @@ SourceRange ConstructorDecl::getSourceRange() const {
   SourceLoc End = getOriginalBodySourceRange().End;
   if (End.isInvalid())
     End = getGenericTrailingWhereClauseSourceRange().End;
-  if (End.isInvalid())
-    End = getThrowsLoc();
   if (End.isInvalid())
     End = getSignatureSourceRange().End;
 
diff --git a/lib/IDE/SyntaxModel.cpp b/lib/IDE/SyntaxModel.cpp
@@ -873,7 +873,7 @@ ASTWalker::PreWalkAction ModelASTWalker::walkToDeclPre(Decl *D) {
     SN.BodyRange = innerCharSourceRangeFromSourceRange(SM,
                                                    AFD->getBodySourceRange());
     SN.NameRange = charSourceRangeFromSourceRange(SM,
-                        AFD->getSignatureSourceRange());
+                        AFD->getParameterListSourceRange());
     if (FD) {
       SN.TypeRange = charSourceRangeFromSourceRange(SM,
                                     FD->getResultTypeSourceRange());
diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
@@ -8409,7 +8409,6 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
     // If this is a call to a distributed method thunk,
     // let's mark the call as implicitly throwing.
     if (isDistributedThunk(callee, apply->getFn())) {
-      auto *FD = cast<AbstractFunctionDecl>(callee.getDecl());
       apply->setImplicitlyThrows(true);
     }
 
diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp
@@ -2545,7 +2545,7 @@ InterfaceTypeRequest::evaluate(Evaluator &eval, ValueDecl *D) const {
     if (thrownTy) {
       thrownTy = AFD->getThrownInterfaceType();
       ProtocolDecl *errorProto = Context.getErrorDecl();
-      if (thrownTy && errorProto) {
+      if (thrownTy && !thrownTy->hasError() && errorProto) {
         Type thrownTyInContext = AFD->mapTypeIntoContext(thrownTy);
         if (!checkConformance(thrownTyInContext, errorProto)) {
           SourceLoc loc;
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
@@ -4278,12 +4278,15 @@ NeverNullType TypeResolver::resolveASTFunctionType(
     thrownTy = resolveType(thrownTypeRepr, thrownTypeOptions);
     if (thrownTy->hasError()) {
       thrownTy = Type();
-    } else if (!options.contains(TypeResolutionFlags::SilenceErrors) &&
-               !thrownTy->hasTypeParameter() &&
-               !checkConformance(thrownTy, ctx.getErrorDecl())) {
-      diagnoseInvalid(
-          thrownTypeRepr, thrownTypeRepr->getLoc(), diag::thrown_type_not_error,
-          thrownTy);
+    } else if (inStage(TypeResolutionStage::Interface) &&
+               !options.contains(TypeResolutionFlags::SilenceErrors)) {
+      auto thrownTyInContext = GenericEnvironment::mapTypeIntoContext(
+        resolution.getGenericSignature().getGenericEnvironment(), thrownTy);
+      if (!checkConformance(thrownTyInContext, ctx.getErrorDecl())) {
+        diagnoseInvalid(
+            thrownTypeRepr, thrownTypeRepr->getLoc(), diag::thrown_type_not_error,
+            thrownTy);
+      }
     }
   }
 
diff --git a/test/Generics/typed_throws.swift b/test/Generics/typed_throws.swift
@@ -14,7 +14,7 @@ func f2<T: P1>(_: T) where () throws(T.A) -> () == () throws -> () {}
 
 protocol P2 {
   associatedtype A where A == () throws(E) -> ()
-  associatedtype E
+  associatedtype E: Error
 }
 
 // CHECK-LABEL: typed_throws.(file).f3@
diff --git a/test/IDE/structure.swift b/test/IDE/structure.swift
@@ -42,11 +42,18 @@ struct MyStruct {
   static func cfoo()
 }
 
+enum MyError: Error {
+  case x
+}
+
 // CHECK: <protocol>protocol <name>MyProt</name> {
 // CHECK:   <ifunc>func <name>foo()</name></ifunc>
 // CHECK:   <ifunc>func <name>foo2()</name> throws</ifunc>
 // CHECK:   <ifunc>func <name>foo3()</name> throws -> <type>Int</type></ifunc>
-// CHECK:   <ifunc>func <name>foo4<<generic-param><name>T</name></generic-param>>()</name> where T: MyProt</ifunc>
+
+// FIXME: The end of the source range needs to be advanced past the ')' here!
+// CHECK:   <ifunc>func <name>foo4()</name> throws(MyError</ifunc>)
+// CHECK:   <ifunc>func <name>foo5<<generic-param><name>T</name></generic-param>>()</name> where T: MyProt</ifunc>
 // CHECK:   <ifunc><name>init()</name></ifunc>
 // CHECK:   <ifunc><name>init(<param><name>a</name>: <type>Int</type></param>)</name> throws</ifunc>
 // 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 {
   func foo()
   func foo2() throws
   func foo3() throws -> Int
-  func foo4<T>() where T: MyProt
+  func foo4() throws(MyError)
+  func foo5<T>() where T: MyProt
   init()
   init(a: Int) throws
   init<T>(a: T) where T: MyProt
diff --git a/test/decl/func/typed_throws.swift b/test/decl/func/typed_throws.swift
@@ -1,5 +1,9 @@
 // RUN: %target-typecheck-verify-swift -swift-version 5 -module-name test
 
+// Don't complain about <<error type>> not conforming to Error
+func invalidThrownType() throws(DoesNotExist) {}
+// expected-error@-1 {{cannot find type 'DoesNotExist' in scope}}
+
 // expected-note@+1{{type declared here}}
 enum MyError: Error {
   case fail
@@ -207,3 +211,8 @@ extension Result {
     }
   }
 }
+
+struct NotAnError<T> {}
+
+func badThrowingFunctionType<T>(_: () throws(NotAnError<T>) -> ()) {}
+// expected-error@-1 {{thrown type 'NotAnError<T>' does not conform to the 'Error' protocol}}
diff --git a/test/decl/protocol/typed_throws.swift b/test/decl/protocol/typed_throws.swift
@@ -0,0 +1,10 @@
+// RUN: %target-typecheck-verify-swift
+
+// Make sure that the source range of a protocol requirement
+// with no body still includes the thrown type, by checking
+// that the name lookup from there finds the generic parameter.
+
+protocol TypedThrowsProto {
+    init<E>(y: () throws(E) -> Void) throws(E)
+    func f<E>(y: () throws(E) -> Void) throws(E)
+}
diff --git a/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp b/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp
@@ -890,17 +890,17 @@ static void setLocationInfo(const ValueDecl *VD,
 
   auto Loc = VD->getLoc(/*SerializedOK=*/true);
   if (Loc.isValid()) {
-    auto getSignatureRange =
+    auto getParameterListRange =
         [&](const ValueDecl *VD) -> std::optional<unsigned> {
       if (auto FD = dyn_cast<AbstractFunctionDecl>(VD)) {
-        SourceRange R = FD->getSignatureSourceRange();
+        SourceRange R = FD->getParameterListSourceRange();
         if (R.isValid())
           return getCharLength(SM, R);
       }
       return std::nullopt;
     };
     unsigned NameLen;
-    if (auto SigLen = getSignatureRange(VD)) {
+    if (auto SigLen = getParameterListRange(VD)) {
       NameLen = SigLen.value();
     } else if (VD->hasName()) {
       NameLen = VD->getBaseName().userFacingName().size();

Original file line number	Diff line number	Diff line change
`@@ -8409,7 +8409,6 @@ Expr ExprRewriter::finishApply(ApplyExpr apply, Type openedType,`
`8409`	`8409`	`// If this is a call to a distributed method thunk,`
`8410`	`8410`	`// let's mark the call as implicitly throwing.`
`8411`	`8411`	`if (isDistributedThunk(callee, apply->getFn())) {`
`8412`		`- auto *FD = cast<AbstractFunctionDecl>(callee.getDecl());`
`8413`	`8412`	`apply->setImplicitlyThrows(true);`
`8414`	`8413`	`}`
`8415`	`8414`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ func f2<T: P1>(_: T) where () throws(T.A) -> () == () throws -> () {}`
`14`	`14`
`15`	`15`	`protocol P2 {`
`16`	`16`	`associatedtype A where A == () throws(E) -> ()`
`17`		`- associatedtype E`
	`17`	`+ associatedtype E: Error`
`18`	`18`	`}`
`19`	`19`
`20`	`20`	`// CHECK-LABEL: typed_throws.(file).f3@`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,9 @@`
`1`	`1`	`// RUN: %target-typecheck-verify-swift -swift-version 5 -module-name test`
`2`	`2`
	`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`	`+`
`3`	`7`	`// expected-note@+1{{type declared here}}`
`4`	`8`	`enum MyError: Error {`
`5`	`9`	`case fail`
`@@ -207,3 +211,8 @@ extension Result {`
`207`	`211`	`}`
`208`	`212`	`}`
`209`	`213`	`}`
	`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}}`