system/serenity
system/serenity
1
0
Fork 0
mirror of https://github.com/SerenityOS/serenity synced 2024-07-22 02:26:11 +00:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

JSSpecCompiler: Recurse into the correct subtrees in RecursiveASTVisitor

Browse source 
RecursiveASTVisitor was recursing into the subtrees of an old root if it
was changed in on_entry callback. Fix that by querying root pointer just
after on_entry callback returns. While on it, also use
`AK::TemporaryChange` instead of setting `m_current_subtree_pointer`
manually.

As it turns out, `FunctionCallCanonicalizationPass` was relying on being
able to replace tree on entry, and the bug in RecursiveASTVisitor made
the pass to not fully canonicalize nested function calls.

The changes to GenericASTPass.cpp alone are enough to fix the problem
but it is canonical (for some definition of canonicity) to only change
trees in on_leave. Therefore, the commit also switches
FunctionCallCanonicalizationPass to on_leave callback.

A test for this fix and one from the previous commit is also included.
This commit is contained in:
Dan Klishch 2024-01-16 01:07:00 -05:00 committed by Andrew Kaster
parent 33b36476d9
commit b4a9fde756
6 changed files with 77 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/GenericASTPass.cpp
View file

@ -4,8 +4,10 @@
* SPDX-License-Identifier: BSD-2-Clause
*/
#include "Compiler/GenericASTPass.h"
#include <AK/TemporaryChange.h>
#include "AST/AST.h"
#include "Compiler/GenericASTPass.h"
#include "Function.h"
namespace JSSpecCompiler {
@ -34,10 +36,10 @@ void RecursiveASTVisitor::replace_current_node_with(NullableTree tree)
RecursionDecision RecursiveASTVisitor::recurse(Tree root, NodeSubtreePointer& pointer)
{
RecursionDecision decision;
TemporaryChange change { m_current_subtree_pointer, &pointer };
m_current_subtree_pointer = &pointer;
decision = on_entry(root);
RecursionDecision decision = on_entry(root);
root = pointer.get({});
if (decision == RecursionDecision::Recurse) {
for (auto& child : root->subtrees()) {
@ -46,7 +48,6 @@ RecursionDecision RecursiveASTVisitor::recurse(Tree root, NodeSubtreePointer& po
}
}
m_current_subtree_pointer = &pointer;
on_leave(root);
return RecursionDecision::Continue;

5
Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.cpp
View file

@ -9,9 +9,9 @@
namespace JSSpecCompiler {
RecursionDecision FunctionCallCanonicalizationPass::on_entry(Tree tree)
void FunctionCallCanonicalizationPass::on_leave(Tree tree)
{
if (auto binary_operation = as<BinaryOperation>(tree); binary_operation) {
if (auto binary_operation = as<BinaryOperation>(tree)) {
if (binary_operation->m_operation == BinaryOperator::FunctionCall) {
Vector<Tree> arguments;
@ -33,7 +33,6 @@ RecursionDecision FunctionCallCanonicalizationPass::on_entry(Tree tree)
replace_current_node_with(make_ref_counted<FunctionCall>(binary_operation->m_left, move(arguments)));
}
}
return RecursionDecision::Recurse;
}
}

2
Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.h
View file

@ -22,7 +22,7 @@ public:
using GenericASTPass::GenericASTPass;
protected:
RecursionDecision on_entry(Tree tree) override;
void on_leave(Tree tree) override;
};
}

35
Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Tests/spec-single-function-simple.xml Normal file
View file

@ -0,0 +1,35 @@
<!DOCTYPE inline_dtd[<!ENTITY nbsp " ">]>
<specification>
<emu-import href="spec/celestial.html">
<emu-clause id="sec-celestial-now-object">
<h1><span class="secnum">2</span> The Celestial.Now Object</h1>
<emu-clause id="sec-celestial-now-abstract-ops">
<h1><span class="secnum">2.3</span> Abstract Operations</h1>
<emu-clause id="sec-celestial-systemutcepochmilliseconds" aoid="SystemUTCEpochMilliseconds">
<h1><span class="secnum">2.3.2</span> SystemUTCEpochMilliseconds ( )</h1>
<emu-alg>
<ol>
<li>
Let <var>global</var> be
<emu-xref aoid="GetGlobalObject"><a href="https://tc39.es/ecma262/#sec-getglobalobject">GetGlobalObject</a></emu-xref>
().
</li>
<li>
Let <var>nowNs</var> be
<emu-xref aoid="HostSystemUTCEpochNanoseconds" id="_ref_119"><a href="#sec-hostsystemutcepochnanoseconds">HostSystemUTCEpochNanoseconds</a></emu-xref>
(<var>global</var>).
</li>
<li>
Return
<emu-xref aoid="𝔽"><a href="https://tc39.es/ecma262/#𝔽">𝔽</a></emu-xref>
(
<emu-xref aoid="floor"><a href="https://tc39.es/ecma262/#eqn-floor">floor</a></emu-xref>
(<var>nowNs</var> / 1000000)).
</li>
</ol>
</emu-alg>
</emu-clause>
</emu-clause>
</emu-clause>
</emu-import>
</specification>

21
Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Tests/spec-single-function-simple.xml.expectation Normal file
View file

@ -0,0 +1,21 @@
===== AST after reference-resolving =====
SystemUTCEpochMilliseconds():
TreeList
BinaryOperation Assignment
Var global
FunctionCall
UnresolvedReference GetGlobalObject
BinaryOperation Assignment
Var nowNs
FunctionCall
UnresolvedReference HostSystemUTCEpochNanoseconds
Var global
ReturnNode
FunctionCall
UnresolvedReference 𝔽
FunctionCall
UnresolvedReference floor
BinaryOperation Division
Var nowNs
MathematicalConstant 1000000

12
Tests/JSSpecCompiler/test-runner.cpp
View file

@ -36,11 +36,23 @@ constexpr TestDescription::Flag always_dump_all = {
.dump_cfg = true
};
constexpr TestDescription::Flag dump_after_frontend = {
.name = "reference-resolving"sv,
.dump_ast = true,
.dump_cfg = false
};
const Array regression_tests = {
TestDescription {
.sources = { "simple.cpp"sv },
.flags = { always_dump_all },
},
TestDescription {
.sources = {
"spec-single-function-simple.xml"sv,
},
.flags = { dump_after_frontend },
}
};
static const LexicalPath path_to_compiler_binary = [] {