Skip to content

Commit 4bfa3b2

Browse files
slavapestovslavapestov
authored
Merge pull request #9377 from slavapestov/fix-some-bugs
Fix nested types in closure context and non-static operator crash
2 parents 9f9945f + d0105f5 commit 4bfa3b2

19 files changed

+139
-79
lines changed

‎include/swift/AST/DeclContext.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,9 @@ class alignas(1 << DeclContextAlignInBits) DeclContext {
363363
return ParentAndKind.getPointer();
364364
}
365365

366+
/// Returns the semantic parent for purposes of name lookup.
367+
DeclContext *getParentForLookup() const;
368+
366369
/// Return true if this is a child of the specified other decl context.
367370
bool isChildContextOf(const DeclContext *Other) const {
368371
if (this == Other) return false;

‎lib/AST/DeclContext.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,16 @@ Decl *DeclContext::getInnermostDeclarationDeclContext() {
406406
return nullptr;
407407
}
408408

409+
DeclContext *DeclContext::getParentForLookup() const {
410+
if (isa<ProtocolDecl>(this) || isa<ExtensionDecl>(this)) {
411+
// If we are inside a protocol or an extension, skip directly
412+
// to the module scope context, without looking at any (invalid)
413+
// outer types.
414+
return getModuleScopeContext();
415+
}
416+
return getParent();
417+
}
418+
409419
ModuleDecl *DeclContext::getParentModule() const {
410420
const DeclContext *DC = this;
411421
while (!DC->isModuleContext())

‎lib/AST/NameLookup.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -857,7 +857,7 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
857857
}
858858
}
859859

860-
DC = DC->getParent();
860+
DC = DC->getParentForLookup();
861861
}
862862

863863
if (!isCascadingUse.hasValue())

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4733,35 +4733,24 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
47334733
// Check for static/final/class when we're in a type.
47344734
auto dc = FD->getDeclContext();
47354735
if (dc->isTypeContext()) {
4736-
// Within a class, operator functions must be 'static' or 'final'.
4737-
if (auto classDecl = dc->getAsClassOrClassExtensionContext()) {
4738-
// For a class, we also need the function or class to be 'final'.
4739-
if (!classDecl->isFinal() && !FD->isFinal() &&
4740-
FD->getStaticSpelling() != StaticSpellingKind::KeywordStatic) {
4741-
if (!FD->isStatic()) {
4742-
TC.diagnose(FD->getLoc(), diag::nonstatic_operator_in_type,
4743-
operatorName,
4744-
dc->getDeclaredInterfaceType())
4745-
.fixItInsert(FD->getAttributeInsertionLoc(/*forModifier=*/true),
4746-
"static ");
4747-
4748-
FD->setStatic();
4749-
} else {
4750-
TC.diagnose(FD->getLoc(), diag::nonfinal_operator_in_class,
4751-
operatorName, dc->getDeclaredInterfaceType())
4752-
.fixItInsert(FD->getAttributeInsertionLoc(/*forModifier=*/true),
4753-
"final ");
4754-
FD->getAttrs().add(new (TC.Context) FinalAttr(/*IsImplicit=*/true));
4755-
}
4756-
}
4757-
} else if (!FD->isStatic()) {
4758-
// Operator functions must be static.
4759-
TC.diagnose(FD, diag::nonstatic_operator_in_type,
4736+
if (!FD->isStatic()) {
4737+
TC.diagnose(FD->getLoc(), diag::nonstatic_operator_in_type,
47604738
operatorName,
47614739
dc->getDeclaredInterfaceType())
47624740
.fixItInsert(FD->getAttributeInsertionLoc(/*forModifier=*/true),
47634741
"static ");
4742+
47644743
FD->setStatic();
4744+
} else if (auto classDecl = dc->getAsClassOrClassExtensionContext()) {
4745+
// For a class, we also need the function or class to be 'final'.
4746+
if (!classDecl->isFinal() && !FD->isFinal() &&
4747+
FD->getStaticSpelling() != StaticSpellingKind::KeywordStatic) {
4748+
TC.diagnose(FD->getLoc(), diag::nonfinal_operator_in_class,
4749+
operatorName, dc->getDeclaredInterfaceType())
4750+
.fixItInsert(FD->getAttributeInsertionLoc(/*forModifier=*/true),
4751+
"final ");
4752+
FD->getAttrs().add(new (TC.Context) FinalAttr(/*IsImplicit=*/true));
4753+
}
47654754
}
47664755
} else if (!dc->isModuleScopeContext()) {
47674756
TC.diagnose(FD, diag::operator_in_local_scope);

‎lib/Sema/TypeCheckType.cpp

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -259,33 +259,22 @@ findDeclContextForType(TypeChecker &TC,
259259

260260
auto ownerDC = typeDecl->getDeclContext();
261261

262-
// If the type is declared at the top level, there's nothing we can learn from
263-
// walking our parent contexts.
264-
if (ownerDC->isModuleScopeContext())
262+
// If the type is not nested in another type, there's nothing we can
263+
// learn from walking parent contexts.
264+
if (!ownerDC->isTypeContext() ||
265+
isa<GenericTypeParamDecl>(typeDecl))
265266
return std::make_tuple(Type(), true);
266267

267-
// Workaround for issue where generic typealias generic parameters are
268-
// looked up with the wrong 'fromDC'.
269-
if (isa<TypeAliasDecl>(ownerDC)) {
270-
assert(isa<GenericTypeParamDecl>(typeDecl));
271-
return std::make_tuple(Type(), true);
272-
}
273-
274-
bool needsBaseType = (ownerDC->isTypeContext() &&
275-
!isa<GenericTypeParamDecl>(typeDecl));
276268
NominalTypeDecl *ownerNominal =
277269
ownerDC->getAsNominalTypeOrNominalTypeExtensionContext();
278270

279271
// We might have an invalid extension that didn't resolve.
280272
//
281273
// FIXME: How did UnqualifiedLookup find the decl then?
282-
if (needsBaseType && ownerNominal == nullptr)
274+
if (ownerNominal == nullptr)
283275
return std::make_tuple(Type(), false);
284276

285277
auto getSelfType = [&](DeclContext *DC) -> Type {
286-
if (!needsBaseType)
287-
return Type();
288-
289278
// When looking up a nominal type declaration inside of a
290279
// protocol extension, always use the nominal type and
291280
// not the protocol 'Self' type.
@@ -330,18 +319,6 @@ findDeclContextForType(TypeChecker &TC,
330319
}
331320

332321
// We're going to check the next parent context.
333-
334-
// FIXME: Horrible hack. Don't allow us to reference a generic parameter
335-
// from a context outside a ProtocolDecl.
336-
if (isa<ProtocolDecl>(parentDC) && isa<GenericTypeParamDecl>(typeDecl))
337-
return std::make_tuple(Type(), false);
338-
}
339-
340-
// If we didn't find the member in an immediate parent context and
341-
// there is no base type, something went wrong.
342-
if (!needsBaseType) {
343-
assert(false && "Should have found non-type context by now");
344-
return std::make_tuple(Type(), false);
345322
}
346323

347324
// Now, search the supertypes or refined protocols of each parent
@@ -447,11 +424,6 @@ findDeclContextForType(TypeChecker &TC,
447424
// If not, walk into the refined protocols, if any.
448425
pushRefined(protoDecl);
449426
}
450-
451-
// FIXME: Horrible hack. Don't allow us to reference a generic parameter
452-
// or associated type from a context outside a ProtocolDecl.
453-
if (isa<ProtocolDecl>(parentDC) && isa<AbstractTypeParamDecl>(typeDecl))
454-
return std::make_tuple(Type(), false);
455427
}
456428

457429
assert(false && "Should have found context by now");
@@ -943,7 +915,7 @@ static Type diagnoseUnknownType(TypeChecker &tc, DeclContext *dc,
943915
// Try ignoring access control.
944916
DeclContext *lookupDC = dc;
945917
if (options.contains(TR_GenericSignature))
946-
lookupDC = dc->getParent();
918+
lookupDC = dc->getParentForLookup();
947919

948920
NameLookupOptions relookupOptions = lookupOptions;
949921
relookupOptions |= NameLookupFlags::KnownPrivate;
@@ -1232,8 +1204,7 @@ resolveTopLevelIdentTypeComponent(TypeChecker &TC, DeclContext *DC,
12321204
if (!DC->isCascadingContextForLookup(/*excludeFunctions*/false))
12331205
options |= TR_KnownNonCascadingDependency;
12341206

1235-
// The remaining lookups will be in the parent context.
1236-
lookupDC = DC->getParent();
1207+
lookupDC = DC->getParentForLookup();
12371208
}
12381209

12391210
// We need to be able to perform unqualified lookup into the given

‎test/Constraints/tuple.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,3 +213,22 @@ var y = 0
213213
let _ = (x, (y, 0))
214214
takesRValue((x, (y, 0)))
215215
takesAny((x, (y, 0)))
216+
217+
// SR-2600 - Closure cannot infer tuple parameter names
218+
typealias Closure<A, B> = ((a: A, b: B)) -> String
219+
220+
func invoke<A, B>(a: A, b: B, _ closure: Closure<A,B>) {
221+
print(closure((a, b)))
222+
}
223+
224+
invoke(a: 1, b: "B") { $0.b }
225+
226+
invoke(a: 1, b: "B") { $0.1 }
227+
228+
invoke(a: 1, b: "B") { (c: (a: Int, b: String)) in
229+
return c.b
230+
}
231+
232+
invoke(a: 1, b: "B") { c in
233+
return c.b
234+
}

‎test/decl/func/operator.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ class C0 {
228228
}
229229

230230
class C1 {
231-
final func %%%(lhs: C1, rhs: C1) -> C1 { return lhs }
231+
final func %%%(lhs: C1, rhs: C1) -> C1 { return lhs } // expected-error{{operator '%%%' declared in type 'C1' must be 'static'}}{{3-3=static }}
232232
}
233233

234234
final class C2 {

‎test/decl/nested/protocol.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@ struct OuterGeneric<D> {
77
protocol InnerProtocol { // expected-error{{protocol 'InnerProtocol' cannot be nested inside another declaration}}
88
associatedtype Rooster
99
func flip(_ r: Rooster)
10-
func flop(_ t: D)
10+
func flop(_ t: D) // expected-error{{use of undeclared type 'D'}}
1111
}
1212
}
1313

1414
class OuterGenericClass<T> {
1515
protocol InnerProtocol { // expected-error{{protocol 'InnerProtocol' cannot be nested inside another declaration}}
1616
associatedtype Rooster
1717
func flip(_ r: Rooster)
18-
func flop(_ t: T)
18+
func flop(_ t: T) // expected-error{{use of undeclared type 'T'}}
1919
}
2020
}
2121

@@ -25,7 +25,7 @@ protocol OuterProtocol {
2525
// expected-note@-1 {{did you mean 'InnerProtocol'?}}
2626
associatedtype Rooster
2727
func flip(_ r: Rooster)
28-
func flop(_ h: Hen)
28+
func flop(_ h: Hen) // expected-error{{use of undeclared type 'Hen'}}
2929
}
3030
}
3131

‎test/decl/nested/type_in_function.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,23 @@
33
// Generic class locally defined in non-generic function (rdar://problem/20116710)
44
func f3() {
55
class B<T> {}
6+
7+
class C : B<Int> {}
8+
9+
_ = B<Int>()
10+
_ = C()
11+
}
12+
13+
// Type defined inside a closure (rdar://problem/31803589)
14+
func hasAClosure() {
15+
_ = {
16+
enum E<T> { case a(T) }
17+
18+
let _ = E.a("hi")
19+
let _ = E<String>.a("hi")
20+
let _: E = .a("hi")
21+
let _: E<String> = .a("hi")
22+
}
623
}
724

825
protocol Racoon {

‎validation-test/IDE/crashers_2/0010-reference-to-self-in-extension-init.swift

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎validation-test/IDE/crashers_2_fixed/0010-reference-to-self-in-extension-init.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// RUN: %target-swift-ide-test -code-completion -code-completion-token=A -source-filename=%s
2+
3+
extension Integer {
4+
#^A^#
5+
extension {
6+
var : Self

‎validation-test/compiler_crashers_2_fixed/0094-rdar30689883.swift

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// RUN: not %target-swift-frontend %s -emit-ir
2+
3+
public struct Trie<Key: Hashable, Value> {
4+
5+
internal var root: TrieNode<Key, Value>
6+
7+
public init() {
8+
self.root = TrieNode<Key, Value>()
9+
}
10+
11+
public mutating func removeValue<C: Collection>(forCollection collection: C) -> Value? where C.Iterator.Element == Key {
12+
return self.root.removeValue(forIterator: collection.makeIterator())
13+
}
14+
15+
public mutating func updateValue<C: Collection>(_ value: Value, forCollection collection: C) -> Value? where C.Iterator.Element == Key {
16+
return self.root.updateValue(value, forIterator: collection.makeIterator())
17+
}
18+
19+
public func value<C: Collection>(forCollection collection: C) -> Value? where C.Iterator.Element == Key {
20+
return self.root.value(forIterator: collection.makeIterator())
21+
}
22+
}
23+
24+
internal struct TrieNode<Key: Hashable, Value> {
25+
26+
internal let children: Dictionary<Key, TrieNode<Key, Value>>
27+
28+
internal let value: Value?
29+
30+
internal init(value: Value?=nil) {
31+
self.children = Dictionary<Key, TrieNode<Key, Value>>()
32+
self.value = value
33+
}
34+
35+
internal mutating func removeValue<I: IteratorProtocol>(forIterator iterator: inout I) -> Value? where I.Element == Key {
36+
return nil
37+
}
38+
39+
internal mutating func updateValue<I: IteratorProtocol>(_ value: Value, forIterator iterator: inout I) -> Value? where I.Element == Key {
40+
return nil
41+
}
42+
43+
internal func value<I: IteratorProtocol>(forIterator iterator: inout I) -> Value? where I.Element == Key {
44+
guard let key = iterator.next() else {
45+
return value
46+
}
47+
48+
return self.children[key]?.value(forIterator: iterator)
49+
}
50+
}
51+
52+

‎validation-test/compiler_crashers/28701-false-should-have-found-context-by-now.swift renamed to ‎validation-test/compiler_crashers_fixed/28701-false-should-have-found-context-by-now.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@
66
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

88
// REQUIRES: asserts
9-
// RUN: not --crash %target-swift-frontend %s -emit-ir
9+
// RUN: not %target-swift-frontend %s -emit-ir
1010
protocol P{class a}class C:P{protocol A:a

‎validation-test/compiler_crashers/28725-gpdecl-getdepth-generictypeparamdecl-invaliddepth-parameter-hasnt-been-validated.swift renamed to ‎validation-test/compiler_crashers_fixed/28725-gpdecl-getdepth-generictypeparamdecl-invaliddepth-parameter-hasnt-been-validated.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@
66
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

88
// REQUIRES: asserts
9-
// RUN: not --crash %target-swift-frontend %s -emit-ir
9+
// RUN: not %target-swift-frontend %s -emit-ir
1010
struct B{let f=a}protocol P{}extension P{extension{func&(U=

‎validation-test/compiler_crashers/28727-objectty-haserror-cannot-have-errortype-wrapped-inside-lvaluetype.swift renamed to ‎validation-test/compiler_crashers_fixed/28727-objectty-haserror-cannot-have-errortype-wrapped-inside-lvaluetype.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@
66
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

88
// REQUIRES: asserts
9-
// RUN: not --crash %target-swift-frontend %s -emit-ir
9+
// RUN: not %target-swift-frontend %s -emit-ir
1010
protocol P}extension P{lazy var f={extension{var f=((self

‎validation-test/compiler_crashers/28729-archetype-bad-generic-context-nesting.swift renamed to ‎validation-test/compiler_crashers_fixed/28729-archetype-bad-generic-context-nesting.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

88
// REQUIRES: asserts
9-
// RUN: not --crash %target-swift-frontend %s -emit-ir
9+
// RUN: not %target-swift-frontend %s -emit-ir
1010
protocol P{
1111
protocol
1212
P{typealias a{}struct B{extension{protocol P{

‎validation-test/compiler_crashers/28734-conformingreplacementtype-is-substitutabletype-conformingreplacementtype-is-depe.swift renamed to ‎validation-test/compiler_crashers_fixed/28734-conformingreplacementtype-is-substitutabletype-conformingreplacementtype-is-depe.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

88
// REQUIRES: asserts
9-
// RUN: not --crash %target-swift-frontend %s -emit-ir
9+
// RUN: not %target-swift-frontend %s -emit-ir
1010
protocol P{typealias a
1111
struct A{{}func a:a
1212
{

‎validation-test/compiler_crashers/28735-reftype-hastypeparameter-cannot-have-a-dependent-type-here.swift renamed to ‎validation-test/compiler_crashers_fixed/28735-reftype-hastypeparameter-cannot-have-a-dependent-type-here.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@
66
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

88
// REQUIRES: asserts
9-
// RUN: not --crash %target-swift-frontend %s -emit-ir
9+
// RUN: not %target-swift-frontend %s -emit-ir
1010
protocol A{protocol A:A{class a{let c=a}typealias a

‎validation-test/compiler_crashers/28741-anonymous-namespace-verifier-walktodeclpost-swift-decl.swift renamed to ‎validation-test/compiler_crashers_fixed/28741-anonymous-namespace-verifier-walktodeclpost-swift-decl.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
// See https://swift.org/LICENSE.txt for license information
66
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

8-
// RUN: not --crash %target-swift-frontend %s -emit-ir
8+
// RUN: not %target-swift-frontend %s -emit-ir
99
protocol A{{}struct A{typealias a:Self
1010
protocol P{extension{lazy var f=A.a

0 commit comments

Comments
 (0)