Skip to content

Commit 71c4c16

Browse files
author
Nathan Hawes
authored
Merge pull request #26270 from nathawes/53313197-syntaxmodel-crash
[IDE] Fix SyntaxModel crash due to out-of-order walking of EnumElementDecls
2 parents 1586105 + f23c24f commit 71c4c16

File tree

9 files changed

+111
-52
lines changed

9 files changed

+111
-52
lines changed

‎lib/IDE/SyntaxModel.cpp

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ class ModelASTWalker : public ASTWalker {
248248

249249
Optional<SyntaxNode> parseFieldNode(StringRef Text, StringRef OrigText,
250250
SourceLoc OrigLoc);
251-
llvm::DenseSet<ASTNode> VisitedNodesInsideIfConfig;
251+
llvm::DenseSet<ASTNode> NodesVisitedBefore;
252252
/// When non-zero, we should avoid passing tokens as syntax nodes since a parent of several tokens
253253
/// is considered as one, e.g. object literal expression.
254254
uint8_t AvoidPassingSyntaxToken = 0;
@@ -310,8 +310,8 @@ class ModelASTWalker : public ASTWalker {
310310
bool searchForURL(CharSourceRange Range);
311311
bool findFieldsInDocCommentLine(SyntaxNode Node);
312312
bool findFieldsInDocCommentBlock(SyntaxNode Node);
313-
bool isVisitedBeforeInIfConfig(ASTNode Node) {
314-
return VisitedNodesInsideIfConfig.count(Node) > 0;
313+
bool isVisitedBefore(ASTNode Node) {
314+
return NodesVisitedBefore.count(Node) > 0;
315315
}
316316
};
317317

@@ -417,7 +417,7 @@ void ModelASTWalker::visitSourceFile(SourceFile &SrcFile,
417417
}
418418

419419
std::pair<bool, Expr *> ModelASTWalker::walkToExprPre(Expr *E) {
420-
if (isVisitedBeforeInIfConfig(E))
420+
if (isVisitedBefore(E))
421421
return {false, E};
422422

423423
auto addCallArgExpr = [&](Expr *Elem, TupleExpr *ParentTupleExpr) {
@@ -568,7 +568,7 @@ Expr *ModelASTWalker::walkToExprPost(Expr *E) {
568568
}
569569

570570
std::pair<bool, Stmt *> ModelASTWalker::walkToStmtPre(Stmt *S) {
571-
if (isVisitedBeforeInIfConfig(S)) {
571+
if (isVisitedBefore(S)) {
572572
return {false, S};
573573
}
574574
auto addExprElem = [&](SyntaxStructureElementKind K, const Expr *Elem,
@@ -710,7 +710,7 @@ Stmt *ModelASTWalker::walkToStmtPost(Stmt *S) {
710710
}
711711

712712
bool ModelASTWalker::walkToDeclPre(Decl *D) {
713-
if (isVisitedBeforeInIfConfig(D))
713+
if (isVisitedBefore(D))
714714
return false;
715715
if (D->isImplicit())
716716
return false;
@@ -850,7 +850,7 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
850850
} else {
851851
Element.get<Decl*>()->walk(*this);
852852
}
853-
VisitedNodesInsideIfConfig.insert(Element);
853+
NodesVisitedBefore.insert(Element);
854854
}
855855
}
856856

@@ -888,13 +888,6 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
888888
SourceRange NameRange = SourceRange(EnumElemD->getNameLoc(),
889889
ParamList->getSourceRange().End);
890890
SN.NameRange = charSourceRangeFromSourceRange(SM, NameRange);
891-
892-
for (auto Param : ParamList->getArray()) {
893-
auto TL = Param->getTypeLoc();
894-
CharSourceRange TR = charSourceRangeFromSourceRange(SM,
895-
TL.getSourceRange());
896-
passNonTokenNode({SyntaxNodeKind::TypeId, TR});
897-
}
898891
} else {
899892
SN.NameRange = CharSourceRange(EnumElemD->getNameLoc(),
900893
EnumElemD->getName().getLength());
@@ -906,7 +899,8 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
906899
charSourceRangeFromSourceRange(SM, ElemRange));
907900
}
908901
pushStructureNode(SN, EnumElemD);
909-
popStructureNode();
902+
EnumElemD->walk(*this);
903+
NodesVisitedBefore.insert(EnumElemD);
910904
}
911905
}
912906
} else if (auto *TypeAliasD = dyn_cast<TypeAliasDecl>(D)) {
@@ -1039,12 +1033,17 @@ bool ModelASTWalker::handleSpecialDeclAttribute(const DeclAttribute *D,
10391033
if (!Arg->walk(*this))
10401034
return false;
10411035
}
1042-
} else {
1036+
} else if (!TokenNodes.empty()) {
10431037
auto Next = TokenNodes.front();
1044-
TokenNodes = TokenNodes.drop_front();
1045-
assert(Next.Range.getStart() == D->getRangeWithAt().Start);
1046-
if (!passNode({SyntaxNodeKind::AttributeBuiltin, Next.Range}))
1047-
return false;
1038+
if (Next.Range.getStart() == D->getRangeWithAt().Start) {
1039+
TokenNodes = TokenNodes.drop_front();
1040+
if (!passNode({SyntaxNodeKind::AttributeBuiltin, Next.Range}))
1041+
return false;
1042+
} else {
1043+
assert(0 && "Attribute's TokenNodes already consumed?");
1044+
}
1045+
} else {
1046+
assert(0 && "No TokenNodes?");
10481047
}
10491048
if (!passTokenNodesUntil(D->getRange().End, PassNodesBehavior::IncludeNodeAtLocation))
10501049
return false;
@@ -1177,7 +1176,7 @@ bool ModelASTWalker::passNonTokenNode(const SyntaxNode &Node) {
11771176
// such as multiple PatternBindingDecl in code like: var a, b : Int. Which
11781177
// would cause us to report the TypeRepr twice.
11791178
if (!SM.isBeforeInBuffer(LastLoc, Node.Range.getStart()))
1180-
return false;
1179+
return true;
11811180

11821181
if (!passTokenNodesUntil(Node.Range.getStart(), DisplaceNodeAtLocation))
11831182
return false;

‎test/IDE/coloring.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,15 @@ struct S {
1818
var a, b : Int
1919
}
2020

21+
enum EnumWhereCaseHasADefaultedFunctionTypeParam {
22+
// CHECK: <kw>enum</kw> EnumWhereCaseHasADefaultedFunctionTypeParam {
23+
case foo(x: () -> () = {
24+
// CHECK: <kw>case</kw> foo(x: () -> () = {
25+
func inner(x: S) {}
26+
// CHECK: <kw>func</kw> inner(x: <type>S</type>) {}
27+
})
28+
}
29+
2130
enum EnumWithDerivedEquatableConformance : Int {
2231
// CHECK-LABEL: <kw>enum</kw> EnumWithDerivedEquatableConformance : {{(<type>)}}Int{{(</type>)?}} {
2332
case CaseA

‎test/IDE/coloring_invalid.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// RUN: %target-swift-ide-test -syntax-coloring -source-filename %s | %FileCheck %s
2+
3+
public enum Result {
4+
// CHECK: <attr-builtin>public</attr-builtin> <kw>enum</kw> Result {
5+
case success(a b = {
6+
// CHECK: <kw>case</kw> success(a b = {
7+
8+
@available(*)
9+
// CHECK: <attr-builtin>@available</attr-builtin>(*)
10+
func materialize
11+
// CHECK: <kw>func</kw> materialize

‎test/IDE/structure.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,3 +293,15 @@ myFunc(foo: 0,
293293
bar: baz == 0)
294294
// CHECK: <call><name>myFunc</name>(<arg><name>foo</name>: 0</arg>,
295295
// CHECK: <arg><name>bar</name>: baz == 0</arg>)</call>
296+
297+
298+
enum FooEnum {
299+
// CHECK: <enum>enum <name>FooEnum</name> {
300+
case blah(x: () -> () = {
301+
// CHECK: <enum-case>case <enum-elem><name>blah(<param><name>x</name>: <type>() -> ()</type> = <closure><brace>{
302+
@Tuples func foo(x: MyStruc) {}
303+
// CHECK: @Tuples <ffunc>func <name>foo(<param><name>x</name>: <type>MyStruc</type></param>)</name> {}</ffunc>
304+
})
305+
// CHECK: }</brace></closure></param>)</name></enum-elem></enum-case>
306+
}
307+
// CHECK: }</enum>

‎test/SourceKit/DocumentStructure/structure.swift.empty.response

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,18 +1269,20 @@
12691269
key.offset: 2011,
12701270
key.length: 13,
12711271
key.nameoffset: 2011,
1272-
key.namelength: 13
1272+
key.namelength: 13,
1273+
key.substructure: [
1274+
{
1275+
key.kind: source.lang.swift.decl.var.parameter,
1276+
key.name: "arg",
1277+
key.offset: 2015,
1278+
key.length: 8,
1279+
key.typename: "Int",
1280+
key.nameoffset: 2015,
1281+
key.namelength: 3
1282+
}
1283+
]
12731284
}
12741285
]
1275-
},
1276-
{
1277-
key.kind: source.lang.swift.decl.var.parameter,
1278-
key.name: "arg",
1279-
key.offset: 2015,
1280-
key.length: 8,
1281-
key.typename: "Int",
1282-
key.nameoffset: 2015,
1283-
key.namelength: 3
12841286
}
12851287
]
12861288
},

‎test/SourceKit/DocumentStructure/structure.swift.foobar.response

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,18 +1269,20 @@
12691269
key.offset: 2011,
12701270
key.length: 13,
12711271
key.nameoffset: 2011,
1272-
key.namelength: 13
1272+
key.namelength: 13,
1273+
key.substructure: [
1274+
{
1275+
key.kind: source.lang.swift.decl.var.parameter,
1276+
key.name: "arg",
1277+
key.offset: 2015,
1278+
key.length: 8,
1279+
key.typename: "Int",
1280+
key.nameoffset: 2015,
1281+
key.namelength: 3
1282+
}
1283+
]
12731284
}
12741285
]
1275-
},
1276-
{
1277-
key.kind: source.lang.swift.decl.var.parameter,
1278-
key.name: "arg",
1279-
key.offset: 2015,
1280-
key.length: 8,
1281-
key.typename: "Int",
1282-
key.nameoffset: 2015,
1283-
key.namelength: 3
12841286
}
12851287
]
12861288
},

‎test/SourceKit/DocumentStructure/structure.swift.response

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,18 +1269,20 @@
12691269
key.offset: 2011,
12701270
key.length: 13,
12711271
key.nameoffset: 2011,
1272-
key.namelength: 13
1272+
key.namelength: 13,
1273+
key.substructure: [
1274+
{
1275+
key.kind: source.lang.swift.decl.var.parameter,
1276+
key.name: "arg",
1277+
key.offset: 2015,
1278+
key.length: 8,
1279+
key.typename: "Int",
1280+
key.nameoffset: 2015,
1281+
key.namelength: 3
1282+
}
1283+
]
12731284
}
12741285
]
1275-
},
1276-
{
1277-
key.kind: source.lang.swift.decl.var.parameter,
1278-
key.name: "arg",
1279-
key.offset: 2015,
1280-
key.length: 8,
1281-
key.typename: "Int",
1282-
key.nameoffset: 2015,
1283-
key.namelength: 3
12841286
}
12851287
]
12861288
},

‎test/SourceKit/InterfaceGen/gen_swift_source.swift.response

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,26 @@ internal enum Colors {
595595
key.attribute: source.decl.attribute.internal
596596
}
597597
]
598+
},
599+
{
600+
key.kind: source.lang.swift.decl.var.instance,
601+
key.accessibility: source.lang.swift.accessibility.internal,
602+
key.setter_accessibility: source.lang.swift.accessibility.internal,
603+
key.name: "p2",
604+
key.offset: 559,
605+
key.length: 24,
606+
key.typename: "(Int, Int)",
607+
key.nameoffset: 568,
608+
key.namelength: 2,
609+
key.docoffset: 527,
610+
key.doclength: 19,
611+
key.attributes: [
612+
{
613+
key.offset: 550,
614+
key.length: 8,
615+
key.attribute: source.decl.attribute.internal
616+
}
617+
]
598618
}
599619
]
600620
},

‎tools/SourceKit/lib/SwiftLang/SwiftLangSupport.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,9 @@ UIdent SwiftLangSupport::getUIDForSyntaxNodeKind(SyntaxNodeKind SC) {
467467
return KindObjectLiteral;
468468
}
469469

470-
llvm_unreachable("Unhandled SyntaxNodeKind in switch.");
470+
// Default to a known kind to prevent crashing in non-asserts builds
471+
assert(0 && "Unhandled SyntaxNodeKind in switch.");
472+
return KindIdentifier;
471473
}
472474

473475
UIdent SwiftLangSupport::getUIDForSyntaxStructureKind(

0 commit comments

Comments
 (0)