Merge pull request #65400 from slavapestov/generic-param-shadowing

Sema: Ban shadowing generic parameters from outer scopes

Author: slavapestov

include/swift/AST/DiagnosticsSema.def

@@ -2783,6 +2783,10 @@ ERROR(requires_generic_params_made_equal,none,
 ERROR(requires_generic_param_made_equal_to_concrete,none,
       "same-type requirement makes generic parameter %0 non-generic",
       (Type))
+ERROR(shadowed_generic_param,none,
+      "generic parameter %0 shadows generic parameter from outer scope with the same name",
+      (DeclName))
+
 ERROR(recursive_decl_reference,none,
       "%0 %1 references itself", (DescriptiveDeclKind, DeclBaseName))
 ERROR(recursive_generic_signature,none,

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -454,11 +454,10 @@ static void diagnoseDuplicateDecls(T &&decls) {
         other->diagnose(diag::invalid_redecl_prev, other->getName());
       });
 
-      // Mark the decl as invalid, unless it's a GenericTypeParamDecl, which is
-      // expected to maintain its type of GenericTypeParamType.
-      // This is needed to avoid emitting a duplicate diagnostic when running
-      // redeclaration checking in the case where the VarDecl is part of the
-      // enclosing context, e.g `let (x, x) = (0, 0)`.
+      // Mark the decl as invalid. This is needed to avoid emitting a
+      // duplicate diagnostic when running redeclaration checking in
+      // the case where the VarDecl is part of the enclosing context,
+      // e.g `let (x, x) = (0, 0)`.
       if (!isa<GenericTypeParamDecl>(current))
         current->setInvalid();
     }
@@ -492,7 +491,7 @@ static void checkGenericParams(GenericContext *ownerCtx) {
                          [](Requirement, RequirementRepr *) { return false; });
 
   // Check for duplicate generic parameter names.
-  diagnoseDuplicateDecls(*genericParams);
+  TypeChecker::checkShadowedGenericParams(ownerCtx);
 }
 
 template <typename T>

lib/Sema/TypeCheckGeneric.cpp

@@ -498,6 +498,63 @@ void TypeChecker::checkReferencedGenericParams(GenericContext *dc) {
   }
 }
 
+/// Ensure we don't re-declare any generic parameters in the current scope,
+/// or shadow a generic parameter from an outer scope.
+void TypeChecker::checkShadowedGenericParams(GenericContext *dc) {
+  // Collect all outer generic parameters for lookup.
+  llvm::SmallDenseMap<Identifier, GenericTypeParamDecl *, 4> genericParamDecls;
+  for (auto *parentDC = dc->getParent(); parentDC != nullptr;
+       parentDC = parentDC->getParentForLookup()) {
+    if (auto *extensionDecl = dyn_cast<ExtensionDecl>(parentDC)) {
+      parentDC = extensionDecl->getExtendedNominal();
+
+      // This can happen with invalid code.
+      if (parentDC == nullptr)
+        return;
+    }
+    if (auto *parentDecl = parentDC->getAsDecl()) {
+      if (auto *parentGeneric = parentDecl->getAsGenericContext()) {
+        if (auto *genericParamList = parentGeneric->getGenericParams()) {
+          for (auto *genericParamDecl : genericParamList->getParams()) {
+            if (genericParamDecl->isOpaqueType())
+              continue;
+            genericParamDecls[genericParamDecl->getName()] = genericParamDecl;
+          }
+        }
+      }
+    }
+  }
+
+  for (auto *genericParamDecl : dc->getGenericParams()->getParams()) {
+    if (genericParamDecl->isOpaqueType() || genericParamDecl->isImplicit())
+      continue;
+
+    auto found = genericParamDecls.find(genericParamDecl->getName());
+    if (found != genericParamDecls.end()) {
+      auto *existingParamDecl = found->second;
+
+      if (existingParamDecl->getDeclContext() == dc) {
+        genericParamDecl->diagnose(
+            diag::invalid_redecl,
+            genericParamDecl->getName());
+      } else {
+        genericParamDecl->diagnose(
+            diag::shadowed_generic_param,
+            genericParamDecl->getName()).warnUntilSwiftVersion(6);
+      }
+
+      if (existingParamDecl->getLoc()) {
+        existingParamDecl->diagnose(diag::invalid_redecl_prev,
+                                    existingParamDecl->getName());
+      }
+
+      continue;
+    }
+
+    genericParamDecls[genericParamDecl->getName()] = genericParamDecl;
+  }
+}
+
 ///
 /// Generic types
 ///

lib/Sema/TypeChecker.h

@@ -500,6 +500,10 @@ void checkProtocolSelfRequirements(ValueDecl *decl);
 /// declaration's type, otherwise we have no way to infer them.
 void checkReferencedGenericParams(GenericContext *dc);
 
+/// Ensure we don't re-declare any generic parameters in the current scope,
+/// or shadow a generic parameter from an outer scope.
+void checkShadowedGenericParams(GenericContext *dc);
+
 /// Diagnose a requirement failure.
 ///
 /// \param errorLoc The location at which an error shall be emitted.

test/AutoDiff/Sema/derivative_attr_type_checking.swift

@@ -496,7 +496,7 @@ extension Struct {
   }
 
   // expected-note @+1 {{candidate subscript does not have a setter}}
-  subscript<T: Differentiable>(x: T) -> T { x }
+  subscript<U: Differentiable>(x: U) -> U { x }
 }
 extension Struct where T: Differentiable & AdditiveArithmetic {
   @derivative(of: subscript.get)
@@ -534,14 +534,14 @@ extension Struct where T: Differentiable & AdditiveArithmetic {
   }
 
   @derivative(of: subscript(_:).get, wrt: self)
-  func vjpSubscriptGenericGetter<T: Differentiable>(x: T) -> (value: T, pullback: (T.TangentVector) -> TangentVector)   {
+  func vjpSubscriptGenericGetter<U: Differentiable>(x: U) -> (value: U, pullback: (U.TangentVector) -> TangentVector)   {
     return (x, { _ in .zero })
   }
 
   // expected-error @+2 {{a derivative already exists for '_'}}
   // expected-note @-6 {{other attribute declared here}}
   @derivative(of: subscript(_:), wrt: self)
-  func vjpSubscriptGeneric<T: Differentiable>(x: T) -> (value: T, pullback: (T.TangentVector) -> TangentVector)   {
+  func vjpSubscriptGeneric<U: Differentiable>(x: U) -> (value: U, pullback: (U.TangentVector) -> TangentVector)   {
     return (x, { _ in .zero })
   }
 
@@ -576,8 +576,8 @@ extension Struct where T: Differentiable & AdditiveArithmetic {
   // Error: original subscript has no setter.
   // expected-error @+1 {{referenced declaration 'subscript(_:)' could not be resolved}}
   @derivative(of: subscript(_:).set, wrt: self)
-  mutating func vjpSubscriptGeneric_NoSetter<T: Differentiable>(x: T) -> (
-    value: T, pullback: (T.TangentVector) -> TangentVector
+  mutating func vjpSubscriptGeneric_NoSetter<U: Differentiable>(x: U) -> (
+    value: U, pullback: (U.TangentVector) -> TangentVector
   ) {
     return (x, { _ in .zero })
   }

test/Compatibility/accessibility.swift

@@ -516,8 +516,8 @@ public struct PublicGenericIPReq<T: InternalProto> where T: PrivateProto {} // e
 
 public func genericFunc<T: InternalProto>(_: T) {} // expected-error {{function cannot be declared public because its generic parameter uses an internal type}} {}
 public class GenericClass<T: InternalProto> { // expected-error {{generic class cannot be declared public because its generic parameter uses an internal type}}
-  public init<T: PrivateProto>(_: T) {} // expected-error {{initializer cannot be declared public because its generic parameter uses a private type}}
-  public func genericMethod<T: PrivateProto>(_: T) {} // expected-error {{instance method cannot be declared public because its generic parameter uses a private type}}
+  public init<U: PrivateProto>(_: U) {} // expected-error {{initializer cannot be declared public because its generic parameter uses a private type}}
+  public func genericMethod<U: PrivateProto>(_: U) {} // expected-error {{instance method cannot be declared public because its generic parameter uses a private type}}
 }
 public enum GenericEnum<T: InternalProto> { // expected-error {{generic enum cannot be declared public because its generic parameter uses an internal type}}
   case A

test/Constraints/generics.swift

@@ -508,11 +508,12 @@ public struct S5 {
 
 // rdar://problem/24329052 - QoI: call argument archetypes not lining up leads to ambiguity errors
 
-struct S_24329052<T> { // expected-note {{generic parameter 'T' of generic struct 'S_24329052' declared here}}
+struct S_24329052<T> { // expected-note {{generic parameter 'T' of generic struct 'S_24329052' declared here}} expected-note {{'T' previously declared here}}
   var foo: (T) -> Void
   // expected-note@+1 {{generic parameter 'T' of instance method 'bar(_:)' declared here}}
   func bar<T>(_ v: T) { foo(v) }
   // expected-error@-1 {{cannot convert value of type 'T' (generic parameter of instance method 'bar(_:)') to expected argument type 'T' (generic parameter of generic struct 'S_24329052')}}
+  // expected-warning@-2 {{generic parameter 'T' shadows generic parameter from outer scope with the same name; this is an error in Swift 6}}
 }
 
 extension Sequence {
@@ -943,7 +944,7 @@ do {
   struct Outer<T: P_eaf0300ff7a> {
     struct Inner<U> {}
 
-    func container<T>() -> Inner<T> {
+    func container<V>() -> Inner<V> {
       return Inner()
     }
   }

test/Constraints/issue-53296.swift

@@ -20,7 +20,7 @@ final class ViewController<T> {
 
 extension ViewController: ViewDataSource where T == String {
 // expected-note@-1 {{requirement from conditional conformance of 'ViewController<T>' to 'ViewDataSource'}}
-  func foo<T>() -> [T] {
+  func foo<U>() -> [U] {
     return []
   }
 }

test/Constraints/members.swift

@@ -553,8 +553,8 @@ func rdar_48114578() {
   struct S<T> {
     var value: T
 
-    static func valueOf<T>(_ v: T) -> S<T> {
-      return S<T>(value: v)
+    static func valueOf<U>(_ v: U) -> S<U> {
+      return S<U>(value: v)
     }
   }
 

test/Constraints/pack-expansion-expressions.swift

@@ -219,8 +219,8 @@ do {
     }
 
     do {
-      func testRef<each T>() -> (repeat each T, String) { fatalError() }
-      func testResult<each T>() -> (repeat each T) { fatalError() }
+      func testRef<each U>() -> (repeat each U, String) { fatalError() }
+      func testResult<each U>() -> (repeat each U) { fatalError() }
 
       func experiment1<each U>() -> (repeat each U, String) {
         testResult() // Ok