Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: allow easier checking of permanent deoptimizations #12456

Merged
merged 13 commits into from
Apr 30, 2017
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ test-parallel: all
test-valgrind: all
$(PYTHON) tools/test.py --mode=release --valgrind sequential parallel message

test-check-deopts: all
$(PYTHON) tools/test.py --mode=release --check-deopts parallel sequential -J

# Implicitly depends on $(NODE_EXE). We don't depend on it explicitly because
# it always triggers a rebuild due to it being a .PHONY rule. See the comment
# near the build-addons rule for more background.
Expand Down
7 changes: 4 additions & 3 deletions lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,14 @@ Agent.prototype.getName = function getName(options) {
return name;
};

Agent.prototype.addRequest = function addRequest(req, options) {
Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/,
localAddress/*legacy*/) {
// Legacy API: addRequest(req, host, port, localAddress)
if (typeof options === 'string') {
options = {
host: options,
port: arguments[2],
localAddress: arguments[3]
port,
localAddress
};
}

Expand Down
13 changes: 7 additions & 6 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ function ClientRequest(options, cb) {
'Expected "' + expectedProtocol + '"');
}

const defaultPort = options.defaultPort ||
this.agent && this.agent.defaultPort;
var defaultPort = options.defaultPort ||
this.agent && this.agent.defaultPort;

var port = options.port = options.port || defaultPort || 80;
var host = options.host = validateHost(options.hostname, 'hostname') ||
Expand Down Expand Up @@ -226,7 +226,7 @@ function ClientRequest(options, cb) {

var called = false;

const oncreate = (err, socket) => {
var oncreate = (err, socket) => {
if (called)
return;
called = true;
Expand All @@ -238,14 +238,15 @@ function ClientRequest(options, cb) {
this._deferToConnect(null, null, () => this._flush());
};

var newSocket;
if (this.socketPath) {
this._last = true;
this.shouldKeepAlive = false;
const optionsPath = {
var optionsPath = {
path: this.socketPath,
timeout: this.timeout
};
const newSocket = this.agent.createConnection(optionsPath, oncreate);
newSocket = this.agent.createConnection(optionsPath, oncreate);
if (newSocket && !called) {
called = true;
this.onSocket(newSocket);
Expand All @@ -270,7 +271,7 @@ function ClientRequest(options, cb) {
this._last = true;
this.shouldKeepAlive = false;
if (typeof options.createConnection === 'function') {
const newSocket = options.createConnection(options, oncreate);
newSocket = options.createConnection(options, oncreate);
if (newSocket && !called) {
called = true;
this.onSocket(newSocket);
Expand Down
2 changes: 1 addition & 1 deletion lib/_stream_readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ Readable.prototype.unpipe = function(dest) {
}

// try to find the right one.
const index = state.pipes.indexOf(dest);
var index = state.pipes.indexOf(dest);
if (index === -1)
return this;

Expand Down
12 changes: 5 additions & 7 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ function getOptions(options, defaultOptions) {
}

function copyObject(source) {
const target = {};
for (const key in source)
var target = {};
for (var key in source)
target[key] = source[key];
return target;
}
Expand Down Expand Up @@ -320,7 +320,7 @@ fs.existsSync = function(path) {
};

fs.readFile = function(path, options, callback) {
callback = maybeCallback(arguments[arguments.length - 1]);
callback = maybeCallback(callback || options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see why this might indeed be "semver-major-y" but it looks fine to me. The only difference is in the case callback is passed but not as a function at which point options would be passed and would be rethrown if real options passed.

I think a CITGM run would be sufficient here and if that passes I don't think we need to change it or semver-major it.

Also I'm very surprised arguments use only indexed access and length check is performed deopts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's also because arguments was being used in a function where the named arguments were being overwritten. This could be a Crankshaft-specific limitation though, I'm not sure.

options = getOptions(options, { flag: 'r' });

if (handleError((path = getPathFromURL(path)), callback))
Expand Down Expand Up @@ -1216,9 +1216,7 @@ fs.futimesSync = function(fd, atime, mtime) {
binding.futimes(fd, atime, mtime);
};

function writeAll(fd, isUserFd, buffer, offset, length, position, callback_) {
var callback = maybeCallback(arguments[arguments.length - 1]);

function writeAll(fd, isUserFd, buffer, offset, length, position, callback) {
// write(fd, buffer, offset, length, position, callback)
fs.write(fd, buffer, offset, length, position, function(writeErr, written) {
if (writeErr) {
Expand Down Expand Up @@ -1249,7 +1247,7 @@ function writeAll(fd, isUserFd, buffer, offset, length, position, callback_) {
}

fs.writeFile = function(path, data, options, callback) {
callback = maybeCallback(arguments[arguments.length - 1]);
callback = maybeCallback(callback || options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto for comment above.

options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
const flag = options.flag || 'w';

Expand Down
4 changes: 2 additions & 2 deletions lib/internal/cluster/child.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ function shared(message, handle, indexesKey, cb) {
delete handles[key];
delete indexes[indexesKey];
return close.apply(this, arguments);
};
}.bind(handle);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind explaining this one? Typically bind isn't that great at not causing deopts :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad value context for arguments value

See: https://gist.github.com/Hypercubed/89808f3051101a1a97f3

assert(handles[key] === undefined);
handles[key] = handle;
cb(message.errno, handle);
Expand Down Expand Up @@ -192,7 +192,7 @@ function _disconnect(masterInitiated) {
}
}

for (const key in handles) {
for (var key in handles) {
const handle = handles[key];
delete handles[key];
waitingCount++;
Expand Down
12 changes: 6 additions & 6 deletions lib/internal/process/stdio.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function setupStdio() {

switch (tty_wrap.guessHandleType(fd)) {
case 'TTY':
const tty = require('tty');
var tty = require('tty');
stdin = new tty.ReadStream(fd, {
highWaterMark: 0,
readable: true,
Expand All @@ -60,13 +60,13 @@ function setupStdio() {
break;

case 'FILE':
const fs = require('fs');
var fs = require('fs');
stdin = new fs.ReadStream(null, { fd: fd, autoClose: false });
break;

case 'PIPE':
case 'TCP':
const net = require('net');
var net = require('net');

// It could be that process has been started with an IPC channel
// sitting on fd=0, in such case the pipe for this fd is already
Expand Down Expand Up @@ -152,20 +152,20 @@ function createWritableStdioStream(fd) {

switch (tty_wrap.guessHandleType(fd)) {
case 'TTY':
const tty = require('tty');
var tty = require('tty');
stream = new tty.WriteStream(fd);
stream._type = 'tty';
break;

case 'FILE':
const fs = require('internal/fs');
var fs = require('internal/fs');
stream = new fs.SyncWriteStream(fd, { autoClose: false });
stream._type = 'fs';
break;

case 'PIPE':
case 'TCP':
const net = require('net');
var net = require('net');
stream = new net.Socket({
fd: fd,
readable: false,
Expand Down
18 changes: 9 additions & 9 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,9 @@ class URL {
if (typeof depth === 'number' && depth < 0)
return opts.stylize('[Object]', 'special');

const ctor = getConstructorOf(this);
var ctor = getConstructorOf(this);

const obj = Object.create({
var obj = Object.create({
constructor: ctor === null ? URL : ctor
});

Expand Down Expand Up @@ -879,20 +879,20 @@ class URLSearchParams {
if (typeof recurseTimes === 'number' && recurseTimes < 0)
return ctx.stylize('[Object]', 'special');

const separator = ', ';
const innerOpts = Object.assign({}, ctx);
var separator = ', ';
var innerOpts = Object.assign({}, ctx);
if (recurseTimes !== null) {
innerOpts.depth = recurseTimes - 1;
}
const innerInspect = (v) => util.inspect(v, innerOpts);
var innerInspect = (v) => util.inspect(v, innerOpts);

const list = this[searchParams];
const output = [];
var list = this[searchParams];
var output = [];
for (var i = 0; i < list.length; i += 2)
output.push(`${innerInspect(list[i])} => ${innerInspect(list[i + 1])}`);

const colorRe = /\u001b\[\d\d?m/g;
const length = output.reduce(
var colorRe = /\u001b\[\d\d?m/g;
var length = output.reduce(
(prev, cur) => prev + cur.replace(colorRe, '').length + separator.length,
-separator.length
);
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ function getSignalsToNamesMapping() {
return signalsToNamesMapping;

signalsToNamesMapping = Object.create(null);
for (const key in signals) {
for (var key in signals) {
signalsToNamesMapping[signals[key]] = key;
}

Expand Down
33 changes: 17 additions & 16 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ function createServer(options, connectionListener) {
// connect(path, [cb]);
//
function connect() {
const args = new Array(arguments.length);
var args = new Array(arguments.length);
for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
// TODO(joyeecheung): use destructuring when V8 is fast enough
const normalized = normalizeArgs(args);
const options = normalized[0];
const cb = normalized[1];
var normalized = normalizeArgs(args);
var options = normalized[0];
var cb = normalized[1];
debug('createConnection', normalized);
const socket = new Socket(options);
var socket = new Socket(options);

if (options.timeout) {
socket.setTimeout(options.timeout);
Expand Down Expand Up @@ -915,13 +915,13 @@ function internalConnect(


Socket.prototype.connect = function() {
const args = new Array(arguments.length);
var args = new Array(arguments.length);
for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
// TODO(joyeecheung): use destructuring when V8 is fast enough
const normalized = normalizeArgs(args);
const options = normalized[0];
const cb = normalized[1];
var normalized = normalizeArgs(args);
var options = normalized[0];
var cb = normalized[1];
return realConnect.call(this, options, cb);
};

Expand Down Expand Up @@ -1373,19 +1373,19 @@ function listenInCluster(server, address, port, addressType,


Server.prototype.listen = function() {
const args = new Array(arguments.length);
var args = new Array(arguments.length);
for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
// TODO(joyeecheung): use destructuring when V8 is fast enough
const normalized = normalizeArgs(args);
var normalized = normalizeArgs(args);
var options = normalized[0];
const cb = normalized[1];
var cb = normalized[1];

var hasCallback = (cb !== null);
if (hasCallback) {
this.once('listening', cb);
}
const backlogFromArgs =
var backlogFromArgs =
// (handle, backlog) or (path, backlog) or (port, backlog)
toNumber(args.length > 1 && args[1]) ||
toNumber(args.length > 2 && args[2]); // (port, host, backlog)
Expand Down Expand Up @@ -1414,11 +1414,12 @@ Server.prototype.listen = function() {
// ([port][, host][, backlog][, cb]) where port is specified
// or (options[, cb]) where options.port is specified
// or if options.port is normalized as 0 before
var backlog;
if (typeof options.port === 'number' || typeof options.port === 'string') {
if (!isLegalPort(options.port)) {
throw new RangeError('"port" argument must be >= 0 and < 65536');
}
const backlog = options.backlog || backlogFromArgs;
backlog = options.backlog || backlogFromArgs;
// start TCP server listening on host:port
if (options.host) {
lookupAndListen(this, options.port | 0, options.host, backlog,
Expand All @@ -1434,8 +1435,8 @@ Server.prototype.listen = function() {
// (path[, backlog][, cb]) or (options[, cb])
// where path or options.path is a UNIX domain socket or Windows pipe
if (options.path && isPipeName(options.path)) {
const pipeName = this._pipeName = options.path;
const backlog = options.backlog || backlogFromArgs;
var pipeName = this._pipeName = options.path;
backlog = options.backlog || backlogFromArgs;
listenInCluster(this, pipeName, -1, -1,
backlog, undefined, options.exclusive);
return this;
Expand Down
4 changes: 2 additions & 2 deletions lib/readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,8 @@ Interface.prototype._tabComplete = function(lastKeypressWasTab) {
}

// If there is a common prefix to all matches, then apply that portion.
const f = completions.filter(function(e) { if (e) return e; });
const prefix = commonPrefix(f);
var f = completions.filter(function(e) { if (e) return e; });
var prefix = commonPrefix(f);
if (prefix.length > completeOn.length) {
self._insertString(prefix.slice(completeOn.length));
}
Expand Down
4 changes: 2 additions & 2 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -601,10 +601,10 @@ REPLServer.prototype.createContext = function() {
}
}

const module = new Module('<repl>');
var module = new Module('<repl>');
module.paths = Module._resolveLookupPaths('<repl>', parentModule, true) || [];

const require = internalModule.makeRequireFunction(module);
var require = internalModule.makeRequireFunction(module);
context.module = module;
context.require = require;

Expand Down
2 changes: 1 addition & 1 deletion lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ function formatArray(ctx, value, recurseTimes, visibleKeys, keys) {
}
visibleLength++;
}
const remaining = value.length - index;
var remaining = value.length - index;
if (remaining > 0) {
output.push(`... ${remaining} more item${remaining > 1 ? 's' : ''}`);
}
Expand Down
3 changes: 2 additions & 1 deletion test/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,8 @@ exports.allowGlobals = allowGlobals;
function leakedGlobals() {
const leaked = [];

for (const val in global)
// eslint-disable-next-line no-var
for (var val in global)
if (!knownGlobals.includes(global[val]))
leaked.push(val);

Expand Down
File renamed without changes.
Loading