GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Adds the Transform Flags concept for tree transformations by rbuckton · Pull Request #6983 · microsoft/TypeScript

Conversation

@rbuckton

@rbuckton Ron Buckton (rbuckton) commented Feb 9, 2016

Copy link
Copy Markdown
Contributor

To ensure tree transformations in the TypeScript compiler are efficient, we compute information about various required transformations for each node. Transform flags are aggregated in a bottom-up fashion, so that we already have information about a node's subtree when we compute the transform flags for the node. These transform flags are initially computed during the bind phase of the compiler, as at that point we guarantee a full walk of the tree.

transformflags

The values in the TransformFlags enum fall into several categories:

  • Facts - State something about a node or its subtree. Facts that start with "Contains" indicate that a node somewhere within the current node's subtree requires a transformation. Facts that do not start with "Contains" indicate that the current node itself requires a transformation.
    • Markers - A special class of Facts that indicate specific transformations that need to occur higher in the source tree.
  • Assertions - Sets of Facts to apply to a node. These are used primarily in the computeTransformFlagsForNode function in binder.ts.
  • Exclusions - Bitmasks that are used to prevent some Facts from being copied up the subtree.

As a result, the Node interface is extended to include two additional properties:

  • transformFlags - Contains the TransformFlags that pertain to this specific node.
  • excludeTransformFlags - Contains a bitmask of TransformFlags that should be excluded from this node's transformFlags when aggregating the transform flags of a subtree containing this node.

The binder is modified to compute the transform flags for each node as it walks the tree. As a performance optimization, we skip transform flag aggregation for ambient nodes, type nodes, and declaration files.

Each transformation phase will likely introduce new nodes. As a result, the visitNode and visitNodes functions in visitor.ts perform supplemental aggregation of transform flags to ensure successive transformation phases have the requisite information to perform additional transformations.

For example, it is possible that during the transformation from TypeScript to ES6 we may introduce an arrow function a for more idiomatic ES6 emit. This may then result in a new this capture which we would then need to know about during the next transformation. Rather than send the new tree through another pass of the checker, we will instead leverage the TransformFlags.ContainsLexicalThis and TransformFlags.ContainsCapturedLexicalThis flags to determine whether we need further transformation.

These flags will enable us to have more efficient transformers in the long term, that can make decisions about whether to continue to walk a subtree based on querying these flags. The following example builds on the one found in #6892:

function visitor(node: Node) {
  if (node.transformFlags & TransformFlags.ES7) {
    // This node is an ES7 node and needs a specific transformation
    return visitorWorker(node);
  }
  else if (node.transformFlags & TransformFlags.ContainsES7) {
    // This node contains an ES7 node somewhere in its subtree, so we must walk the subtree
    return visitEachChild(node, visitor);
  }
  else {
    // Neither this node, nor its subtree, require any transformation, so we can simply 
    // reuse the node and its subtree and avoid a costly walk
    return node;
  }
}

function visitorWorker(node: Node) {
  switch (node.kind) {
    case SyntaxKind.BinaryExpression:
      return visitBinaryExpression(<BinaryExpression>node);
    ...
    default:
      return visitEachChild(node, visitor);
  }
}

function visitBinaryExpression(node: BinaryExpression) {
  if (node.operatorToken.kind === SyntaxKind.AsteriskAsteriskToken) {
    return createCall(
      createPropertyAccess(createIdentifier("Math"), "pow"),
      [
        visitNode(node.left, visitor, isExpressionNode),
        visitNode(node.right, visitor, isExpressionNode)
      ]
    );
  }
  else {
    return visitEachChild(node, visitor);
  }
}

Related Pull Requests:

  • Adds the visitor API foundation for tree transformations. #6892 - Adds the visitor API foundation for tree transformations.
  • Adds the Transform Flags concept for tree transformations #6983 - Adds the Transform Flags concept for tree transformations.
  • Adds the transformFiles API for tree transformations #6986 - Adds the transformFiles API for tree transformations.
  • Adds a simplified pretty printer for tree transformations #6987 - Adds a simplified pretty printer for tree transformations.
  • Adds the TypeScript transformer #7191 - Adds the TypeScript transformer.
  • Adds the JSX transformer #7192 - Adds the JSX transformer.
  • Adds the ES7 transformer #7193 - Adds the ES7 transformer.
  • Adds the ES6 transformer #7194 - Adds the ES6 transformer.
  • Adds the Module transformers #7206 - Adds the Module transformer.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Ron Buckton (rbuckton) added 6 commits February 4, 2016 11:57
@rbuckton
Adds transform flags aggregation to binder and visitor
cadda66
@rbuckton
Merge branch 'transforms-visitor' into transforms-flags
ba9181c
@rbuckton
Adds the transformFiles API
1e35593
@rbuckton
Merge branch 'transforms-visitor' into transforms-flags
34489a4
@rbuckton
Merge branch 'transforms-flags' into transforms-transformer
cbb910a
@rbuckton
Merge branch 'transforms-visitor' into transforms-flags
1a0b877
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Feb 9, 2016
@rbuckton

Ron Buckton (rbuckton) commented Feb 9, 2016

Copy link
Copy Markdown
Contributor Author

Paging for review: Mohamed Hegazy (@mhegazy), Anders Hejlsberg (@ahejlsberg), Yui (@yuit), Daniel Rosenwasser (@DanielRosenwasser), Ryan Cavanaugh (@RyanCavanaugh), Vladimir Matveev (@vladima)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ron Buckton (rbuckton) added 3 commits February 9, 2016 11:43
@rbuckton
Moved TreansformFlags assertions after markers
51dd27a
@rbuckton
Merge branch 'transforms-flags' into transforms-transformer
6fa4002
@rbuckton
Moved LexicalEnvironment to types.ts, minor fixes
0f2bbb1
@rbuckton Ron Buckton (rbuckton) mentioned this pull request Feb 9, 2016
Adds the transformFiles API for tree transformations #6986
Merged
Ron Buckton (rbuckton) added 3 commits February 9, 2016 13:43
@rbuckton
Added printer
6b381ec
@rbuckton
Fixed linter error
fb19e81
@rbuckton
Merge branch 'transforms-transformer' into transforms-printer
e545f1b
This was referenced Feb 9, 2016
Adds a simplified pretty printer for tree transformations #6987
Merged
Adds the visitor API foundation for tree transformations. #6892
Merged
@rbuckton
Adds destructuring logic and placeholders for each transformer.
49d2d93
RyanCavanaugh
RyanCavanaugh reviewed Feb 10, 2016
View reviewed changes
Comment thread src/compiler/binder.ts
case SyntaxKind.AbstractKeyword:
case SyntaxKind.DeclareKeyword:
case SyntaxKind.AsyncKeyword:
case SyntaxKind.ConstKeyword:

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Feb 10, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Does this trigger false positives for const x = ... variable declarations?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@rbuckton Ron Buckton (rbuckton) Feb 10, 2016

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

No, we don't treat const as a modifier for variable declarations.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@yuit Yui (yuit) Feb 16, 2016

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

how is 'const' has flag of AssertTypeScript should it be under es6?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@rbuckton Ron Buckton (rbuckton) Feb 16, 2016

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The only ES6 const is for VariableDeclarationList, which is not stored as a modifier. The only time const is a Modifier is for a const enum, which is TypeScript only.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Ron Buckton (rbuckton) added 5 commits February 12, 2016 10:49
@rbuckton
Adds the TypeScript transformer
f948b14
@rbuckton
Merge branch 'transforms-visitor' into transforms-flags
4577441
@rbuckton
PR Feedback
319ff61
@rbuckton
Merge branch 'transforms-flags' into transforms-transformer
ceae78b
@rbuckton
PR feedback and cleanup
f8ed021
Ron Buckton (rbuckton) and others added 26 commits March 2, 2016 15:19
@rbuckton
Switch from onBefore/onAfterEmitNode to onEmitNode
ebb4764
@rbuckton
Merge branch 'transforms-printer' into transforms-transformer-ts
e66e51d
@rbuckton
Switched to onEmitNode
8b35af4
@rbuckton
Merge branch 'transforms-transformer-ts' into transforms-transformer-jsx
1fdaf74
@rbuckton
Merge branch 'transforms-transformer-jsx' into transforms-transformer…
593fbd4
…-es7
@rbuckton
Merge branch 'transforms-transformer-es7' into transforms-transformer…
7b28b48
…-es6
@rbuckton
Switched to onEmitNode
1c73818
@rbuckton
Merge branch 'transforms-transformer-es6' into transforms-transformer…
c4a75ba
…-module
@rbuckton
Switched to onEmitNode
72eebdb
@rbuckton
Fixed invalid assertion in ts transformer
1cf183b
@rbuckton
General fixes and cleanup
d89e21a
@rbuckton
Moved modifier related flags to separate enum
5b8cf96
@rbuckton
Removed ModifiersArray
99e6ad8
@rbuckton
Add support for array return values from visitors
47cdfbe
@rbuckton
Removed NodeArrayNode in favor of arrays
2699bf9
@rbuckton
cleaned up assertions and removed flattenNodes.
94018a1
@rbuckton
Fixed perf issue in binder, plus PR feedback
ad0dd4e
@rbuckton
Updated baselines
44ca7d4
@rbuckton
PR Feedback
ae7843d
@rbuckton
Merge pull request #7206 from Microsoft/transforms-transformer-module
3c5f170
Adds the Module transformers
@rbuckton
Merge pull request #7194 from Microsoft/transforms-transformer-es6
07a3d18
Adds the ES6 transformer
@rbuckton
Merge pull request #7193 from Microsoft/transforms-transformer-es7
cc00f4c
Adds the ES7 transformer
@rbuckton
Merge pull request #7192 from Microsoft/transforms-transformer-jsx
829a9df
Adds the JSX transformer
@rbuckton
Merge pull request #7191 from Microsoft/transforms-transformer-ts
ca9148e
Adds the TypeScript transformer
@rbuckton
Merge pull request #6987 from Microsoft/transforms-printer
85ae53d
Adds a simplified pretty printer for tree transformations
@rbuckton
Merge pull request #6986 from Microsoft/transforms-transformer
a212e2b
Adds the transformFiles API for tree transformations
Ron Buckton (rbuckton) added a commit that referenced this pull request Mar 18, 2016
@rbuckton
Merge pull request #6983 from Microsoft/transforms-flags
f9cb493
Adds the Transform Flags concept for tree transformations
@rbuckton
Ron Buckton (rbuckton) merged commit f9cb493 into transforms-visitor Mar 18, 2016
@rbuckton
Ron Buckton (rbuckton) deleted the transforms-flags branch March 18, 2016 23:39
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@rbuckton @yuit @DanielRosenwasser @weswigham @RyanCavanaugh @msftclas
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.