Skip to content

Commit

Permalink
Bug 1642708 - Remove more references to BinAST code r=arai
Browse files Browse the repository at this point in the history
  • Loading branch information
moztcampbell committed Jun 12, 2020
1 parent 0d2aa31 commit d721f78
Show file tree
Hide file tree
Showing 15 changed files with 40 additions and 221 deletions.
5 changes: 0 additions & 5 deletions js/src/builtin/TestingFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -434,11 +434,6 @@ static bool GetBuildConfiguration(JSContext* cx, unsigned argc, Value* vp) {
return false;
}

value = BooleanValue(false);
if (!JS_SetProperty(cx, info, "binast", value)) {
return false;
}

value.setInt32(sizeof(void*));
if (!JS_SetProperty(cx, info, "pointer-byte-size", value)) {
return false;
Expand Down
1 change: 0 additions & 1 deletion js/src/frontend/BytecodeCompiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,6 @@ static bool CompileLazyFunctionImpl(JSContext* cx, Handle<BaseScript*> lazy,
// compiled, because compilation requires full information about the
// function's immediately enclosing scope.
MOZ_ASSERT(lazy->isReadyForDelazification());
MOZ_ASSERT(!lazy->isBinAST());

AutoAssertReportedException assertException(cx);
Rooted<JSFunction*> fun(cx, lazy->function());
Expand Down
6 changes: 3 additions & 3 deletions js/src/frontend/BytecodeCompiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@
*
* ParseContext.h: class ParseContext: Extremely complex class that serves a lot
* of purposes, but it's a single class - essentially no derived classes - so
* it's a little easier to comprehend all at once. (SourceParseContext and
* BinASTParseContext do derive from ParseContext, but they do nothing except
* adjust the constructor's arguments).
* it's a little easier to comprehend all at once. (SourceParseContext does
* derive from ParseContext, but they does nothing except adjust the
* constructor's arguments).
* Note it uses a thing called Nestable, which implements a stack of objects:
* you can push (and pop) instances to a stack (linked list) as you parse
* further into the parse tree. You may push to this stack via calling the
Expand Down
19 changes: 3 additions & 16 deletions js/src/frontend/BytecodeEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5701,13 +5701,7 @@ MOZ_NEVER_INLINE bool BytecodeEmitter::emitFunction(
return false;
}

EmitterMode nestedMode = emitterMode;
if (nestedMode == BytecodeEmitter::LazyFunction) {
MOZ_ASSERT(compilationInfo.sourceObject->source()->hasBinASTSource());
nestedMode = BytecodeEmitter::Normal;
}

BytecodeEmitter bce2(this, parser, funbox, compilationInfo, nestedMode);
BytecodeEmitter bce2(this, parser, funbox, compilationInfo, emitterMode);
if (!bce2.init(funNode->pn_pos)) {
return false;
}
Expand Down Expand Up @@ -10813,15 +10807,8 @@ bool BytecodeEmitter::intoScriptStencil(ScriptStencil* stencil) {
// FunctionBox.
stencil->immutableFlags.setFlag(ImmutableFlags::HasMappedArgsObj,
funbox->hasMappedArgsObj());

// While IsLikelyConstructorWrapper is required to be the same between
// syntax and normal parsing, BinAST cannot ensure this. Work around this by
// using the existing value if this is delazification.
if (emitterMode != BytecodeEmitter::LazyFunction) {
stencil->immutableFlags.setFlag(
ImmutableFlags::IsLikelyConstructorWrapper,
funbox->isLikelyConstructorWrapper());
}
stencil->immutableFlags.setFlag(ImmutableFlags::IsLikelyConstructorWrapper,
funbox->isLikelyConstructorWrapper());
} /* isFunctionBox */

return true;
Expand Down
12 changes: 5 additions & 7 deletions js/src/frontend/FullParseHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ class TokenStreamAnyChars;
enum class SourceKind {
// We are parsing from a text source (Parser.h)
Text,
// We are parsing from a binary source (BinASTParser.h)
Binary,
};

// Parse handler used when generating a full parse tree for all code which the
Expand Down Expand Up @@ -133,11 +131,11 @@ class FullParseHandler {
FOR_EACH_PARSENODE_SUBCLASS(DECLARE_AS)
#undef DECLARE_AS

// The FullParseHandler may be used to create nodes for text sources
// (from Parser.h) or for binary sources (from BinASTParser.h). In the latter
// case, some common assumptions on offsets are incorrect, e.g. in `a + b`,
// `a`, `b` and `+` may be stored in any order. We use `sourceKind()`
// to determine whether we need to check these assumptions.
// The FullParseHandler may be used to create nodes for text sources (from
// Parser.h). With previous binary source formats, some common assumptions on
// offsets are incorrect, e.g. in `a + b`, `a`, `b` and `+` may be stored in
// any order. We use `sourceKind()` to determine whether we need to check
// these assumptions.
SourceKind sourceKind() const { return sourceKind_; }

NameNodeType newName(PropertyName* name, const TokenPos& pos, JSContext* cx) {
Expand Down
7 changes: 2 additions & 5 deletions js/src/frontend/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1905,11 +1905,8 @@ static bool InstantiateScriptStencils(JSContext* cx,
} else if (funbox->isAsmJSModule()) {
MOZ_ASSERT(funbox->function()->isAsmJSNative());
} else if (funbox->function()->isIncomplete()) {
// Lazy functions are generally only allocated in the initial parse. The
// exception to this is BinAST which does not allocate lazy functions
// inside lazy functions until delazification occurs.
MOZ_ASSERT(compilationInfo.lazy == nullptr ||
compilationInfo.lazy->isBinAST());
// Lazy functions are generally only allocated in the initial parse.
MOZ_ASSERT(compilationInfo.lazy == nullptr);

if (!CreateLazyScript(cx, compilationInfo, funbox)) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion js/src/frontend/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ class AutoInParametersOfAsyncFunction;

class MOZ_STACK_CLASS ParserSharedBase : public JS::CustomAutoRooter {
public:
enum class Kind { Parser, BinASTParser };
enum class Kind { Parser };

ParserSharedBase(JSContext* cx, CompilationInfo& compilationInfo, Kind kind);
~ParserSharedBase();
Expand Down
28 changes: 3 additions & 25 deletions js/src/jit-test/jit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,6 @@ def main(argv):
op.add_argument('--test-reflect-stringify', dest="test_reflect_stringify",
help="instead of running tests, use them to test the "
"Reflect.stringify code in specified file")
op.add_argument('--run-binast', action='store_true',
dest="run_binast",
help="By default BinAST testcases encoded from JS "
"testcases are skipped. If specified, BinAST testcases "
"are also executed.")
# --enable-webrender is ignored as it is not relevant for JIT
# tests, but is required for harness compatibility.
op.add_argument('--enable-webrender', action='store_true',
Expand Down Expand Up @@ -227,22 +222,10 @@ def main(argv):
test_list = []
read_all = True

if options.run_binast:
code = 'print(getBuildConfiguration().binast)'
is_binast_enabled = subprocess.check_output(
[js_shell, '-e', code]
).decode(errors='replace')
if not is_binast_enabled.startswith('true'):
print("While --run-binast is specified, BinAST is not enabled.",
file=sys.stderr)
print("BinAST testcases will be skipped.",
file=sys.stderr)
options.run_binast = False

if test_args:
read_all = False
for arg in test_args:
test_list += jittests.find_tests(arg, run_binast=options.run_binast)
test_list += jittests.find_tests(arg)

if options.read_tests:
read_all = False
Expand All @@ -262,7 +245,7 @@ def main(argv):
sys.stderr.write('---\n')

if read_all:
test_list = jittests.find_tests(run_binast=options.run_binast)
test_list = jittests.find_tests()

if options.exclude_from:
with open(options.exclude_from) as fh:
Expand All @@ -274,7 +257,7 @@ def main(argv):
if options.exclude:
exclude_list = []
for exclude in options.exclude:
exclude_list += jittests.find_tests(exclude, run_binast=options.run_binast)
exclude_list += jittests.find_tests(exclude)
test_list = [test for test in test_list
if test not in set(exclude_list)]

Expand Down Expand Up @@ -326,11 +309,6 @@ def main(argv):
for line in f.readlines():
path = line.strip('\n')
ignore.add(path)

binjs_path = path.replace('.js', '.binjs')
# Do not use os.path.join to always use '/'.
ignore.add('binast/nonlazy/{}'.format(binjs_path))
ignore.add('binast/lazy/{}'.format(binjs_path))
options.ignore_timeouts = ignore
except IOError:
sys.exit("Error reading file: " + options.ignore_timeouts)
Expand Down
33 changes: 6 additions & 27 deletions js/src/shell/js.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1440,8 +1440,6 @@ enum FileKind {
FileScript, // UTF-8, directly parsed as such
FileScriptUtf16, // FileScript, but inflate to UTF-16 before parsing
FileModule,
FileBinASTMultipart,
FileBinASTContext,
};

static void ReportCantOpenErrorUnknownEncoding(JSContext* cx,
Expand Down Expand Up @@ -9915,12 +9913,9 @@ static MOZ_MUST_USE bool ProcessArgs(JSContext* cx, OptionParser* op) {
MultiStringRange utf16FilePaths = op->getMultiStringOption('u');
MultiStringRange codeChunks = op->getMultiStringOption('e');
MultiStringRange modulePaths = op->getMultiStringOption('m');
MultiStringRange binASTPaths(nullptr, nullptr);
FileKind binASTFileKind = FileBinASTMultipart;

if (filePaths.empty() && utf16FilePaths.empty() && codeChunks.empty() &&
modulePaths.empty() && binASTPaths.empty() &&
!op->getStringArg("script")) {
modulePaths.empty() && !op->getStringArg("script")) {
// Always use the interactive shell when -i is used. Without -i we let
// Process figure it out based on isatty.
bool forceTTY = op->getBoolOption('i');
Expand Down Expand Up @@ -9950,16 +9945,14 @@ static MOZ_MUST_USE bool ProcessArgs(JSContext* cx, OptionParser* op) {
}

while (!filePaths.empty() || !utf16FilePaths.empty() || !codeChunks.empty() ||
!modulePaths.empty() || !binASTPaths.empty()) {
!modulePaths.empty()) {
size_t fpArgno = filePaths.empty() ? SIZE_MAX : filePaths.argno();
size_t ufpArgno =
utf16FilePaths.empty() ? SIZE_MAX : utf16FilePaths.argno();
size_t ccArgno = codeChunks.empty() ? SIZE_MAX : codeChunks.argno();
size_t mpArgno = modulePaths.empty() ? SIZE_MAX : modulePaths.argno();
size_t baArgno = binASTPaths.empty() ? SIZE_MAX : binASTPaths.argno();

if (fpArgno < ufpArgno && fpArgno < ccArgno && fpArgno < mpArgno &&
fpArgno < baArgno) {
if (fpArgno < ufpArgno && fpArgno < ccArgno && fpArgno < mpArgno) {
char* path = filePaths.front();
if (!Process(cx, path, false, FileScript)) {
return false;
Expand All @@ -9969,8 +9962,7 @@ static MOZ_MUST_USE bool ProcessArgs(JSContext* cx, OptionParser* op) {
continue;
}

if (ufpArgno < fpArgno && ufpArgno < ccArgno && ufpArgno < mpArgno &&
ufpArgno < baArgno) {
if (ufpArgno < fpArgno && ufpArgno < ccArgno && ufpArgno < mpArgno) {
char* path = utf16FilePaths.front();
if (!Process(cx, path, false, FileScriptUtf16)) {
return false;
Expand All @@ -9980,8 +9972,7 @@ static MOZ_MUST_USE bool ProcessArgs(JSContext* cx, OptionParser* op) {
continue;
}

if (ccArgno < fpArgno && ccArgno < ufpArgno && ccArgno < mpArgno &&
ccArgno < baArgno) {
if (ccArgno < fpArgno && ccArgno < ufpArgno && ccArgno < mpArgno) {
const char* code = codeChunks.front();

JS::CompileOptions opts(cx);
Expand All @@ -10005,19 +9996,7 @@ static MOZ_MUST_USE bool ProcessArgs(JSContext* cx, OptionParser* op) {
continue;
}

if (baArgno < fpArgno && baArgno < ufpArgno && baArgno < ccArgno &&
baArgno < mpArgno) {
char* path = binASTPaths.front();
if (!Process(cx, path, false, binASTFileKind)) {
return false;
}

binASTPaths.popFront();
continue;
}

MOZ_ASSERT(mpArgno < fpArgno && mpArgno < ufpArgno && mpArgno < ccArgno &&
mpArgno < baArgno);
MOZ_ASSERT(mpArgno < fpArgno && mpArgno < ufpArgno && mpArgno < ccArgno);

char* path = modulePaths.front();
if (!Process(cx, path, false, FileModule)) {
Expand Down
31 changes: 3 additions & 28 deletions js/src/tests/lib/jittests.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ def __init__(self, path):
# Exit status or error output.
self.expect_crash = False
self.is_module = False
self.is_binast = False
# Reflect.stringify implementation to test
self.test_reflect_stringify = None

Expand Down Expand Up @@ -180,7 +179,6 @@ def copy(self):
t.test_reflect_stringify = self.test_reflect_stringify
t.enable = True
t.is_module = self.is_module
t.is_binast = self.is_binast
t.skip_if_cond = self.skip_if_cond
t.skip_variant_if_cond = self.skip_variant_if_cond
return t
Expand Down Expand Up @@ -242,18 +240,7 @@ def from_file(cls, path, options):
cls.Directives[dir_name] = dir_meta

filename, file_extension = os.path.splitext(path)
if file_extension == '.binjs':
# BinAST does not have an inline comment format, so it's hard
# to parse file-by-file directives. Allow foo.binjs to use foo.dir
# as an adjacent file to specify.
meta_file_name = filename + '.dir'
if os.path.exists(meta_file_name):
meta = cls.find_directives(meta_file_name)
else:
meta = ''
test.is_binast = True
else:
meta = cls.find_directives(path)
meta = cls.find_directives(path)

if meta != '' or dir_meta != '':
meta = meta + dir_meta
Expand Down Expand Up @@ -378,10 +365,6 @@ def command(self, prefix, libdir, moduledir, remote_prefix=None):
cmd += ['--module-load-path', moduledir]
if self.is_module:
cmd += ['--module', path]
elif self.is_binast:
# In builds with BinAST, this will run the test file. In builds without,
# It's a no-op and the tests will silently pass.
cmd += ['-B', path]
elif self.test_reflect_stringify is None:
cmd += ['-f', path]
else:
Expand All @@ -403,24 +386,16 @@ def get_command(self, prefix):
return self.command(prefix, LIB_DIR, MODULE_DIR)


def find_tests(substring=None, run_binast=False):
def find_tests(substring=None):
ans = []
for dirpath, dirnames, filenames in os.walk(TEST_DIR):
dirnames.sort()
filenames.sort()
if dirpath == '.':
continue

if not run_binast:
if os.path.join('binast', 'lazy') in dirpath:
continue
if os.path.join('binast', 'nonlazy') in dirpath:
continue
if os.path.join('binast', 'invalid') in dirpath:
continue

for filename in filenames:
if not (filename.endswith('.js') or filename.endswith('.binjs')):
if not filename.endswith('.js'):
continue
if filename in ('shell.js', 'browser.js'):
continue
Expand Down
8 changes: 1 addition & 7 deletions js/src/vm/HelperThreads.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,7 @@ namespace wasm {
struct Tier2GeneratorTask;
} // namespace wasm

enum class ParseTaskKind {
Script,
Module,
ScriptDecode,
BinAST,
MultiScriptsDecode
};
enum class ParseTaskKind { Script, Module, ScriptDecode, MultiScriptsDecode };

namespace wasm {

Expand Down
4 changes: 1 addition & 3 deletions js/src/vm/JSFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1570,9 +1570,7 @@ static bool DelazifyCanonicalScriptedFunction(JSContext* cx,
size_t sourceLength = lazy->sourceEnd() - lazy->sourceStart();
bool hadLazyScriptData = lazy->hasPrivateScriptData();

if (ss->hasBinASTSource()) {
MOZ_CRASH("Trying to delazify BinAST function in non-BinAST build");
} else {
{
MOZ_ASSERT(ss->hasSourceText());

// Parse and compile the script from source.
Expand Down
Loading

0 comments on commit d721f78

Please sign in to comment.