Skip to content

Commit 2fe8632

Browse files
committed
AST: Fix crashes when name lookup finds declarations outside of a protocol or extension that's nested in another type
When performing a name lookup from inside of a protocol or extension, skip directly to the source file context when we are done visiting the protocol or extension. Otherwise, if we have invalid code where the protocol or extension is nested inside another type, we might find a member whose type contains generic parameters of the outer type; these parameters will not resolve, since we do not model protocols or extensions nested inside generic contexts (yet?). This supercedes an earlier workaround for a similar issue; the new workaround fixes more crashes. This is needed to avoid crasher regressions with an upcoming patch.
1 parent ceff27c commit 2fe8632

14 files changed

+32
-31
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/TypeCheckType.cpp

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -330,11 +330,6 @@ findDeclContextForType(TypeChecker &TC,
330330
}
331331

332332
// 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);
338333
}
339334

340335
// If we didn't find the member in an immediate parent context and
@@ -447,11 +442,6 @@ findDeclContextForType(TypeChecker &TC,
447442
// If not, walk into the refined protocols, if any.
448443
pushRefined(protoDecl);
449444
}
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);
455445
}
456446

457447
assert(false && "Should have found context by now");
@@ -943,7 +933,7 @@ static Type diagnoseUnknownType(TypeChecker &tc, DeclContext *dc,
943933
// Try ignoring access control.
944934
DeclContext *lookupDC = dc;
945935
if (options.contains(TR_GenericSignature))
946-
lookupDC = dc->getParent();
936+
lookupDC = dc->getParentForLookup();
947937

948938
NameLookupOptions relookupOptions = lookupOptions;
949939
relookupOptions |= NameLookupFlags::KnownPrivate;
@@ -1232,8 +1222,7 @@ resolveTopLevelIdentTypeComponent(TypeChecker &TC, DeclContext *DC,
12321222
if (!DC->isCascadingContextForLookup(/*excludeFunctions*/false))
12331223
options |= TR_KnownNonCascadingDependency;
12341224

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

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

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

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

Lines changed: 0 additions & 7 deletions
This file was deleted.
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/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
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=
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{
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)