Skip to content

Commit b9985db

Browse files
committed
[SyntaxParse] Fix memory leak for explicit syntax actions
Add SyntaxParseActions::discardRecordedNode() as a workaround for memory leak during syntax parsing migration.
1 parent 80085e0 commit b9985db

File tree

9 files changed

+43
-6
lines changed

9 files changed

+43
-6
lines changed

include/swift/Parse/HiddenLibSyntaxAction.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ class HiddenLibSyntaxAction : public SyntaxParseActions {
7070
std::pair<size_t, OpaqueSyntaxNode>
7171
lookupNode(size_t lexerOffset, syntax::SyntaxKind kind) override;
7272

73+
void discardRecordedNode(OpaqueSyntaxNode node) override;
74+
7375
OpaqueSyntaxNodeKind getOpaqueKind() override {
7476
return ExplicitAction->getOpaqueKind();
7577
}

include/swift/Parse/ParsedRawSyntaxRecorder.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ class ParsedRawSyntaxRecorder {
6868
ParsedRawSyntaxNode recordEmptyRawSyntaxCollection(syntax::SyntaxKind kind,
6969
SourceLoc loc);
7070

71+
void discardRecordedNode(ParsedRawSyntaxNode &node);
72+
7173
/// Used for incremental re-parsing.
7274
ParsedRawSyntaxNode lookupNode(size_t lexerOffset, SourceLoc loc,
7375
syntax::SyntaxKind kind);

include/swift/Parse/SyntaxParseActions.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@ class SyntaxParseActions {
6161
ArrayRef<OpaqueSyntaxNode> elements,
6262
CharSourceRange range) = 0;
6363

64+
/// Discard raw syntax node.
65+
///
66+
/// FIXME: This breaks invariant that any recorded node will be a part of the
67+
/// result SourceFile syntax. This method is a temporary workaround, and
68+
/// should be removed when we fully migrate to libSyntax parsing.
69+
virtual void discardRecordedNode(OpaqueSyntaxNode node) = 0;
70+
6471
/// Used for incremental re-parsing.
6572
virtual std::pair<size_t, OpaqueSyntaxNode>
6673
lookupNode(size_t lexerOffset, syntax::SyntaxKind kind) {

include/swift/SyntaxParse/SyntaxTreeCreator.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ class SyntaxTreeCreator: public SyntaxParseActions {
6464
ArrayRef<OpaqueSyntaxNode> elements,
6565
CharSourceRange range) override;
6666

67+
void discardRecordedNode(OpaqueSyntaxNode node) override;
68+
6769
std::pair<size_t, OpaqueSyntaxNode>
6870
lookupNode(size_t lexerOffset, syntax::SyntaxKind kind) override;
6971

lib/Parse/HiddenLibSyntaxAction.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,3 +125,12 @@ HiddenLibSyntaxAction::getExplicitNodeFor(OpaqueSyntaxNode node) {
125125
auto hiddenNode = (Node *)node;
126126
return hiddenNode->ExplicitActionNode;
127127
}
128+
129+
void HiddenLibSyntaxAction::discardRecordedNode(OpaqueSyntaxNode opaqueN) {
130+
if (!opaqueN)
131+
return;
132+
auto node = static_cast<Node *>(opaqueN);
133+
if (!areBothLibSyntax())
134+
LibSyntaxAction->discardRecordedNode(node->LibSyntaxNode);
135+
ExplicitAction->discardRecordedNode(node->ExplicitActionNode);
136+
}

lib/Parse/ParsedRawSyntaxRecorder.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ ParsedRawSyntaxRecorder::recordEmptyRawSyntaxCollection(SyntaxKind kind,
112112
return ParsedRawSyntaxNode{kind, tok::unknown, range, n};
113113
}
114114

115+
void ParsedRawSyntaxRecorder::discardRecordedNode(ParsedRawSyntaxNode &node) {
116+
SPActions->discardRecordedNode(node.takeOpaqueNode());
117+
}
118+
115119
ParsedRawSyntaxNode
116120
ParsedRawSyntaxRecorder::lookupNode(size_t lexerOffset, SourceLoc loc,
117121
SyntaxKind kind) {

lib/Parse/SyntaxParsingContext.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ ParsedRawSyntaxNode SyntaxParsingContext::finalizeSourceFile() {
266266
llvm::makeMutableArrayRef(Layout, 2));
267267
}
268268

269-
OpaqueSyntaxNode SyntaxParsingContext::finalizeRoot() {
269+
OpaqueSyntaxNode SyntaxParsingContext::finalizeRoot() {
270270
assert(isTopOfContextStack() && "some sub-contexts are not destructed");
271271
assert(isRoot() && "only root context can finalize the tree");
272272
assert(Mode == AccumulationMode::Root);
@@ -349,11 +349,12 @@ SyntaxParsingContext::~SyntaxParsingContext() {
349349
// Remove all parts in this context.
350350
case AccumulationMode::Discard: {
351351
auto &nodes = getStorage();
352-
for (auto i = nodes.begin()+Offset, e = nodes.end(); i != e; ++i)
353-
if (i->isRecorded()) {
354-
getSyntaxCreator().finalizeNode(i->getOpaqueNode());
355-
i->reset();
356-
}
352+
for (auto i = nodes.begin()+Offset, e = nodes.end(); i != e; ++i) {
353+
// FIXME: This should not be needed. This breaks invariant that any
354+
// recorded node must be a part of result souce syntax tree.
355+
if (i->isRecorded())
356+
getRecorder().discardRecordedNode(*i);
357+
}
357358
nodes.erase(nodes.begin()+Offset, nodes.end());
358359
break;
359360
}

lib/SyntaxParse/SyntaxTreeCreator.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,9 @@ SyntaxTreeCreator::lookupNode(size_t lexerOffset, syntax::SyntaxKind kind) {
173173
raw.resetWithoutRelease();
174174
return {length, opaqueN};
175175
}
176+
177+
void SyntaxTreeCreator::discardRecordedNode(OpaqueSyntaxNode opaqueN) {
178+
if (!opaqueN)
179+
return;
180+
static_cast<RawSyntax *>(opaqueN)->Release();
181+
}

tools/libSwiftSyntaxParser/libSwiftSyntaxParser.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,10 @@ class CLibParseActions : public SyntaxParseActions {
190190
return getNodeHandler()(&node);
191191
}
192192

193+
void discardRecordedNode(OpaqueSyntaxNode node) override {
194+
// FIXME: This method should not be called at all.
195+
}
196+
193197
std::pair<size_t, OpaqueSyntaxNode>
194198
lookupNode(size_t lexerOffset, SyntaxKind kind) override {
195199
auto NodeLookup = getNodeLookup();

0 commit comments

Comments
 (0)