Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Use isSameLine helper #2599

Merged
merged 1 commit into from
May 17, 2017
Merged

Use isSameLine helper #2599

merged 1 commit into from
May 17, 2017

Conversation

ajafff
Copy link
Contributor

@ajafff ajafff commented Apr 18, 2017

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

This PR converts some uses of ts.getLineAndCharacterOfPosition to tsutils.isSameLine as suggested by @andy-hanson in #2591 (comment)

Is there anything you'd like reviewers to focus on?

curly and one-line are not included, because they are not refactored yet

CHANGELOG.md entry:

[no-log]

@@ -60,10 +60,10 @@ interface Options {

function walk(ctx: Lint.WalkContext<Options>): void {
const { sourceFile, options: { multiline } } = ctx;
ts.forEachChild(sourceFile, function cb(node: ts.Node): void {
return ts.forEachChild(sourceFile, function cb(node: ts.Node): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have any evidence that adding return makes it faster?

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 don't have any evidence and am reluctant to write a micro benchmark for this. I doubt the change here will be measurable at all.
It's just my understanding of what's helping the optimizing compiler.

Copy link
Contributor

Choose a reason for hiding this comment

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

Time measurement isn't necessary if you think it means the difference between tail call and not tail call. If return made it into a tail call, then this code would not get a stack overflow:

function f(x) {
    if (x === 0)
        return console.log("done");
    else
        return f(x - 1);
}
f(30000);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andy-hanson You're confusing tail call optimization (something the compiler can do) and tail call elimination (a language feature that the compiler must do).

The latter can be found in the ES Spec as proper tail calls. This feature will enable (almost) infinite recursion and would make your above example run without error. Unfortunately this is very unlikely to land in V8 because of https://github.com/tc39/proposal-ptc-syntax

Tail call opitimization on the other hand can optimize tail calls whether ptc is implemented or not. Unfortunately this does not enable deeper recursion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants