Skip to content

Commit b380cf9

Browse files
author
Nathan Hawes
committed
[AST|ASTWalker] Fix assertion hit walking an invalid AST with a protocol decl inside an extension
The AST walker calling getGenericParams on a protocol decl would eventually call computeNominalType. computeNominalType checks that the protocol (as a nomimnal type decl) appears within a type context, and if so, asks for the SelfNominalTypeDecl of that context. If the context is an extension, that ends up asking for its extended nominal, which is a problem if we're walking a pre-typechecked AST – we hit the assertion "Extension must have already been bound (by bindExtensions)"). Unlike other nominal types, it's not valid Swift for protocols to appear within type contexts, so exclude protocol decls from taking this code path. This results in us providing Type() as the parent type of the produced NominalType, just like we do for protocols inside functions or other invalid contexts. Resolves <rdar://problem/58549036>
1 parent f6e0db5 commit b380cf9

File tree

3 files changed

+19
-16
lines changed

3 files changed

+19
-16
lines changed

lib/AST/ASTWalker.cpp

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,7 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
223223
}
224224

225225
bool visitTypeAliasDecl(TypeAliasDecl *TAD) {
226-
if (TAD->getGenericParams() &&
227-
Walker.shouldWalkIntoGenericParams()) {
228-
226+
if (Walker.shouldWalkIntoGenericParams() && TAD->getGenericParams()) {
229227
if (visitGenericParamList(TAD->getGenericParams()))
230228
return true;
231229
}
@@ -237,8 +235,7 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
237235
}
238236

239237
bool visitOpaqueTypeDecl(OpaqueTypeDecl *OTD) {
240-
if (OTD->getGenericParams() &&
241-
Walker.shouldWalkIntoGenericParams()) {
238+
if (Walker.shouldWalkIntoGenericParams() && OTD->getGenericParams()) {
242239
if (visitGenericParamList(OTD->getGenericParams()))
243240
return true;
244241
}
@@ -272,16 +269,15 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
272269
}
273270

274271
// Visit requirements
275-
if (auto *Protocol = dyn_cast<ProtocolDecl>(NTD)) {
276-
if (auto *WhereClause = Protocol->getTrailingWhereClause()) {
277-
for (auto &Req: WhereClause->getRequirements()) {
278-
if (doIt(Req))
279-
return true;
280-
}
281-
}
282-
}
283272
if (WalkGenerics) {
284-
for (auto Req: NTD->getGenericParams()->getTrailingRequirements()) {
273+
ArrayRef<swift::RequirementRepr> Reqs = None;
274+
if (auto *Protocol = dyn_cast<ProtocolDecl>(NTD)) {
275+
if (auto *WhereClause = Protocol->getTrailingWhereClause())
276+
Reqs = WhereClause->getRequirements();
277+
} else {
278+
Reqs = NTD->getGenericParams()->getTrailingRequirements();
279+
}
280+
for (auto Req: Reqs) {
285281
if (doIt(Req))
286282
return true;
287283
}
@@ -1329,7 +1325,6 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
13291325

13301326
private:
13311327
bool visitGenericParamListIfNeeded(GenericContext *gc) {
1332-
// Must check this first in case extensions have not been bound yet
13331328
if (Walker.shouldWalkIntoGenericParams()) {
13341329
if (auto *params = gc->getGenericParams()) {
13351330
visitGenericParamList(params);

lib/AST/Decl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3613,7 +3613,7 @@ static Type computeNominalType(NominalTypeDecl *decl, DeclTypeKind kind) {
36133613
// Get the parent type.
36143614
Type Ty;
36153615
DeclContext *dc = decl->getDeclContext();
3616-
if (dc->isTypeContext()) {
3616+
if (!isa<ProtocolDecl>(decl) && dc->isTypeContext()) {
36173617
switch (kind) {
36183618
case DeclTypeKind::DeclaredType: {
36193619
auto *nominal = dc->getSelfNominalTypeDecl();

test/IDE/coloring_invalid.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
// RUN: %target-swift-ide-test -syntax-coloring -source-filename %s | %FileCheck %s
22

3+
public enum Foo {}
4+
// CHECK: <attr-builtin>public</attr-builtin> <kw>enum</kw> Foo {}
5+
public extension Foo {
6+
// CHECK: <attr-builtin>public</attr-builtin> <kw>extension</kw> <type>Foo</type> {
7+
protocol Bar {}
8+
// CHECK: <kw>protocol</kw> Bar {}
9+
}
10+
311
public enum Result {
412
// CHECK: <attr-builtin>public</attr-builtin> <kw>enum</kw> Result {
513
case success(a b = {

0 commit comments

Comments
 (0)