Skip to content

No parent pointers in emitter #4010

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

Closed
wants to merge 11 commits into from
Closed

No parent pointers in emitter #4010

wants to merge 11 commits into from

Conversation

rbuckton
Copy link
Member

This change removes most of the emitter's dependency on parent pointers, to facilitate tree transformations that will generate synthesized nodes.

As we descend into the tree, we keep track of the current node stack. Generally this happens automatically as we call emit, however in cases where we have special transformations in the output (such as for ES5 emit of ES6 classes), we must manually push interceding nodes onto the stack to maintain the hierarchy.

To verify node stack consistency, at the top of most functions in the emitter is a call to verifyStackBehavior, which can be used to catch developer errors when adding new functions to the emitter.

export function getEnclosingBlockScopeContainer(node: Node): Node;
export function getEnclosingBlockScopeContainer(node: Node, getAncestorOrSelf: (offset: number) => Node, offset: number): Node;
export function getEnclosingBlockScopeContainer(node: Node, getAncestorOrSelf?: (offset: number) => Node, offset?: number): Node {
let current = getAncestorOrSelf ? getAncestorOrSelf(++offset) : node.parent;
Copy link
Member

Choose a reason for hiding this comment

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

Just looking at it, I'm not sure what the implied behavior of getAncestorOfSelf is doing with offset and why you need to increment it when feeding the value. A (potentially JSDoc) comment would be helpful.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added some detailed remarks about its purpose and usage

* This is done so that nodes like variable declarations and binding elements can return
* a view of their flags that includes the modifiers from their container. i.e. flags like
* export/declare aren't stored on the variable declaration directly, but on the containing
* variable statement (if it has one). Similarly, flags for let/const are store on the
Copy link
Contributor

Choose a reason for hiding this comment

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

are stored*

@@ -788,6 +791,249 @@ namespace ts {
getSignatureConstructor: () => <any>Signature
};

/**
* Creates an object used to navigate the ancestor's of a node by following parent pointers.
Copy link
Member

Choose a reason for hiding this comment

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

ancestors

@DanielRosenwasser
Copy link
Member

@rbuckton I think this is almost ready to go in, can you resolve the merge conflicts?

@DanielRosenwasser
Copy link
Member

This will need another sync-up

@mhegazy
Copy link
Contributor

mhegazy commented Nov 26, 2015

We should close this one.

@rbuckton
Copy link
Member Author

rbuckton commented Dec 1, 2015

Closing this as it is out of date and will be superseded by a different approach.

@rbuckton rbuckton closed this Dec 1, 2015
@rbuckton rbuckton deleted the noParentNodesInEmitter branch December 1, 2015 23:06
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants