Skip to content

Sema: Ban shadowing generic parameters from outer scopes #65400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 5 additions & 6 deletions lib/Sema/TypeCheckDeclPrimary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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>
Expand Down
57 changes: 57 additions & 0 deletions lib/Sema/TypeCheckGeneric.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions test/AutoDiff/Sema/derivative_attr_type_checking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 })
}

Expand Down Expand Up @@ -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 })
}
Expand Down
4 changes: 2 additions & 2 deletions test/Compatibility/accessibility.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions test/Constraints/generics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/issue-53296.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
}
}
4 changes: 2 additions & 2 deletions test/Constraints/members.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/Constraints/pack-expansion-expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/super_method.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class X<T> {
}

class Y<U> : X<Int> {
func otherMethod<U>(_ x: Int, y: U) {
func otherMethod<V>(_ x: Int, y: V) {
super.method(x, y: y)
}
}
Expand Down
17 changes: 17 additions & 0 deletions test/Generics/shadowed_generic_params.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// RUN: %target-typecheck-verify-swift

struct Outer<T, U> { // expected-note 2{{'T' previously declared here}}
struct Inner<V> { // expected-note 2{{'V' previously declared here}}
func foo<T>(_: T) {} // expected-warning {{generic parameter 'T' shadows generic parameter from outer scope with the same name; this is an error in Swift 6}}
func bar<V>(_: V) {} // expected-warning {{generic parameter 'V' shadows generic parameter from outer scope with the same name; this is an error in Swift 6}}
}
}

extension Outer.Inner {
func baz<T>(_: T) {} // expected-warning {{generic parameter 'T' shadows generic parameter from outer scope with the same name; this is an error in Swift 6}}
func quux<V>(_: V) {} // expected-warning {{generic parameter 'V' shadows generic parameter from outer scope with the same name; this is an error in Swift 6}}
}

extension Sequence {
func bing<Self>(_: Self) {} // expected-warning {{generic parameter 'Self' shadows generic parameter from outer scope with the same name; this is an error in Swift 6}}
}
10 changes: 5 additions & 5 deletions test/Sema/accessibility.swift
Original file line number Diff line number Diff line change
Expand Up @@ -935,9 +935,9 @@ package struct PackageGenericIPReq<T: InternalProto> where T: PrivateProto {} //
public func genericFunc<T: InternalProto>(_: T) {} // expected-error {{function cannot be declared public because its generic parameter uses an internal type}} {}
public func genericFuncPackage<T: PackageProto>(_: T) {} // expected-error {{function cannot be declared public because its generic parameter uses a package 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 func genericMethodPackage<T: PackageProto>(_: T) {} // expected-error {{instance method cannot be declared public because its generic parameter uses a package 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 func genericMethodPackage<U: PackageProto>(_: U) {} // expected-error {{instance method cannot be declared public because its generic parameter uses a package type}}
}
public enum GenericEnum<T: InternalProto> { // expected-error {{generic enum cannot be declared public because its generic parameter uses an internal type}}
case A
Expand All @@ -948,8 +948,8 @@ public enum GenericEnumPackage<T: PackageProto> { // expected-error {{generic en

package func packageGenericFunc<T: InternalProto>(_: T) {} // expected-error {{function cannot be declared package because its generic parameter uses an internal type}} {}
package class PackageGenericClassT<T: InternalProto> { // expected-error {{generic class cannot be declared package because its generic parameter uses an internal type}}
package init<T: PrivateProto>(_: T) {} // expected-error {{initializer cannot be declared package because its generic parameter uses a private type}}
package func packageGenericMethod<T: PrivateProto>(_: T) {} // expected-error {{instance method cannot be declared package because its generic parameter uses a private type}}
package init<U: PrivateProto>(_: U) {} // expected-error {{initializer cannot be declared package because its generic parameter uses a private type}}
package func packageGenericMethod<U: PrivateProto>(_: U) {} // expected-error {{instance method cannot be declared package because its generic parameter uses a private type}}
}
package enum PackageGenericEnumT<T: InternalProto> { // expected-error {{generic enum cannot be declared package because its generic parameter uses an internal type}}
case A
Expand Down
2 changes: 1 addition & 1 deletion test/Sema/moveonly_illegal_types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ enum Maybe<T> {
}

struct CerebralValley<T> {
struct GenericBro<T> {}
struct GenericBro<U> {}
struct TechBro {}
}

Expand Down
12 changes: 6 additions & 6 deletions test/attr/attr_override.swift
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ protocol Protocol2 {}
// Base class is generic, derived class is concrete.

class Base1<T> {
func foo<T: Protocol1>(arg: T) {}
func foo<U: Protocol1>(arg: U) {}
}
class Derived1: Base1<Protocol2> {
override func foo<T>(arg: T) {} // Ok?
Expand All @@ -528,14 +528,14 @@ class Base2 {
func foo() {}
}
class Derived2<T>: Base2 {
override func foo<T>(arg: T) {} // expected-error {{method does not override any method from its superclass}}
override func foo<U>(arg: U) {} // expected-error {{method does not override any method from its superclass}}
}

// Base class generic w/ method generic, derived class generic w/ method not
// generic.

class Base3<T> {
func foo<T>(arg: T) {}
func foo<U>(arg: U) {}
}
class Derived3<T>: Base3<T> {
override func foo() {} // expected-error {{method does not override any method from its superclass}}
Expand All @@ -545,10 +545,10 @@ class Derived3<T>: Base3<T> {
// but different requirement.

class Base4<T> {
func foo<T>(arg: T) {}
func foo<U>(arg: U) {}
}
class Derived4<T>: Base4<T> {
override func foo<T: Protocol1>(arg: T) {} // expected-error {{method does not override any method from its superclass}}
override func foo<U: Protocol1>(arg: U) {} // expected-error {{method does not override any method from its superclass}}
}

// Base class not generic w/ method generic, derived class not generic w/ method
Expand Down Expand Up @@ -578,7 +578,7 @@ class Base7 {
func foo<T: Protocol2>(arg: T) {}
}
class Derived7<T>: Base7 {
override func foo<T: Protocol1>(arg: T) {} // expected-error {{method does not override any method from its superclass}}
override func foo<U: Protocol1>(arg: U) {} // expected-error {{method does not override any method from its superclass}}
}

// Override with orthogonal requirement on contextual generic parameter.
Expand Down
2 changes: 1 addition & 1 deletion test/attr/attr_rethrows_protocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ struct ConformsToSimpleThrowsClosure<T : RethrowingProtocol> : SimpleThrowsClosu
try t.source()
}

func doIt2<T : Empty>(_: T) rethrows {}
func doIt2<U : Empty>(_: U) rethrows {}
// expected-note@-1 {{candidate is 'rethrows' via a conformance, but the protocol requirement is not from a '@rethrows' protocol}}
}

Expand Down
2 changes: 1 addition & 1 deletion test/decl/ext/generic.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ extension X<Int, Double, String> {
// expected-error@-1 {{extensions must not contain stored properties}}
static let x = 0
func f() -> Int {}
class C<T> {}
class C<W> {}
}

typealias GGG = X<Int, Double, String>
Expand Down
2 changes: 1 addition & 1 deletion test/decl/nested/type_in_extension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ struct G<T> {}

extension G {
struct H<U> {
func usesBoth<T, U>(t: T, u: U) -> (T, U) {}
func usesBoth<T1, U1>(t: T1, u: U1) -> (T1, U1) {}
}
}

Expand Down
6 changes: 3 additions & 3 deletions test/decl/nested/type_in_function.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ class OuterGenericClass<T> {
init(t: T) { super.init(); self.t = t }
}

class InnerGenericClass<U> : OuterGenericClass<U> // expected-error {{type 'InnerGenericClass' cannot be nested in generic function 'genericFunction'}}
where U : Racoon, U.Stripes == T {
class InnerGenericClass<V> : OuterGenericClass<V> // expected-error {{type 'InnerGenericClass' cannot be nested in generic function 'genericFunction'}}
where V : Racoon, V.Stripes == T {
let t: T

init(t: T) { super.init(); self.t = t }
Expand Down Expand Up @@ -130,7 +130,7 @@ func genericFunction<T>(t: T) {
class First : Second<T>.UnknownType { }
// expected-error@-1 {{type 'First' cannot be nested in generic function 'genericFunction(t:)'}}
// expected-error@-2 {{'UnknownType' is not a member type of generic class 'type_in_function.Second<T>'}}
class Second<T> : Second { } // expected-note{{'Second' declared here}}
class Second<U> : Second { } // expected-note{{'Second' declared here}}
// expected-error@-1 {{type 'Second' cannot be nested in generic function 'genericFunction(t:)'}}
// expected-error@-2 {{'Second' inherits from itself}}
}
Expand Down
4 changes: 2 additions & 2 deletions test/decl/nested/type_in_type.swift
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ class OuterGenericClass<T> {
}

class Middle {
class Inner1<T> {}
class Inner2<T> : Middle where T: Inner1<Int> {}
class Inner1<U> {}
class Inner2<U> : Middle where U: Inner1<Int> {}
}
}

Expand Down
6 changes: 3 additions & 3 deletions test/decl/overload.swift
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,9 @@ struct X5<t, u, v> {
static func u() {}
typealias v = String

func foo<t>(_ t: t) {
let t = t
_ = t
func foo<w>(_ w: w) {
let w = w
_ = w
}
}

Expand Down
Loading