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

Experiment #3

Closed
wants to merge 6 commits into from
Closed

Experiment #3

wants to merge 6 commits into from

Conversation

BurtHarris
Copy link
Collaborator

Don't actually merge this, but it might give you an idea what I'm thinking for the conversion process.

* specified tree.</p>
*/
/*@Override*/
visit(/*@NotNull*/ tree: ParseTree): Result {
Copy link
Member

@sharwell sharwell Oct 3, 2016

Choose a reason for hiding this comment

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

❓ Could we get away with @NotNull (or @notnull) as a decorator here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is experimental support for decorators. We could define a module that defines dummy decorators and leave them in-place. Is that would you are thinking about?

Copy link
Member

@sharwell sharwell Oct 3, 2016

Choose a reason for hiding this comment

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

💭 It'd be nice to leave them in to start with. We can always pull them out later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Easy to change in my script.


export abstract class AbstractParseTreeVisitor<Result> implements ParseTreeVisitor<Result> {
/**
* {@inheritDoc}
Copy link
Member

Choose a reason for hiding this comment

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

❓ Is there any specific syntax/semantics for TypeScript documentation comments, or are they just plain text?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To typescript itself, it's plain text, but I think there are tools for Javadoc like processing...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is a special-case for comments starting with /// <reference... , but it's not something you need to worry about just now.


// ConvertTo-TS run at 2016-10-02T18:54:42.6538504-07:00

export abstract class AbstractParseTreeVisitor<Result> implements ParseTreeVisitor<Result> {
Copy link
Member

Choose a reason for hiding this comment

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

❓ How would this be imported in another file, considering it's in a "tree" subfolder?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably easier for me to give you and example...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For code outside the runtime, you would use a non-relative module identifier. E.g.:

import {AbstractParseTreeVisitor} from 'antlr4/tree';

We also have the option to define a façade, by writing something like the index.ts files, that re-exports all public types in a flattened form, so that this could work:

import {AbstractParseTreeVisitor} from 'antlr4';

I think I would prefer the later, but not too strongly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually the name antlr4 above might be antlr4ts given the current module. We stake-out the ground for a given name by registering it on npmjs.org (as I understand it.).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think Eric has already register antlr4.

Copy link
Member

Choose a reason for hiding this comment

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

antlr4ts makes sense to me


// ConvertTo-TS run at 2016-10-02T18:54:42.7458518-07:00

export interface ErrorNode extends TerminalNode {
Copy link
Member

Choose a reason for hiding this comment

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

❓ In ANTLR, this is essentially a marker interface. Is it possible to perform a type check in TypeScript so you know if an object implements a marker interface, even if it has no members?

Copy link
Collaborator Author

@BurtHarris BurtHarris Oct 3, 2016

Choose a reason for hiding this comment

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

No!. Interfaces in typescript have no existence at runtime. For cases like this we will probably want to change an interface into an abstract class, so that we can use the instanceof operator.

export interface ParseTree extends SyntaxTree {
// the following methods narrow the return type; they are not additional methods
/*@Override*/
getParent(): ParseTree;
Copy link
Member

@sharwell sharwell Oct 3, 2016

Choose a reason for hiding this comment

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

❓ Should this be:

readonly parent: ParseTree;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It could be... I was looking to make the minimum source change necessary, rather than impose TypeScript style on the code, at least at first...

* in your event methods.
*/
export class ParseTreeProperty<V> {
protected annotations: Map<ParseTree, V> = new IdentityHashMap<ParseTree, V>();
Copy link
Member

Choose a reason for hiding this comment

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

📝 This class is intended to work like a decorator. If I'm not mistaken, in TypeScript we can actually add new properties to an existing object. The annotations field simulates that ability when working in Java (a language that does not support setting new properties on an existing object). If I'm correct regarding the capability of TypeScript, then we can eliminate the annotations field and instead store the values on the objects themselves.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea, sort of, I think. Lots of flexibility in adding properties. I need to understand the use-case for this a bit more to discuss intelligently.

Copy link
Member

@sharwell sharwell Oct 3, 2016

Choose a reason for hiding this comment

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

I use it for adding strongly-typed data to ParserRuleContext instances, e.g. in walkers and visitors, without actually modifying my grammar to force them to appear in generated code. It's really a helper class that's not used by anything in the runtime.

Copy link
Member

Choose a reason for hiding this comment

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

For more background, it's conceptually derived from ObjectDecorator<T> and ObjectProperty<T> which I created while working on ANTLRWorks 2. I defined various properties which can be assigned to tree nodes during semantic analysis.

@@ -0,0 +1,14 @@
export * from './Tree';
Copy link
Member

@sharwell sharwell Oct 3, 2016

Choose a reason for hiding this comment

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

❓ So this file is for a person to do the following?

import * from "./tree";

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Almost. You can do

import { Tree } from "./tree";

In typescript you seem to have to explicitly identify each name you want to import.

@BurtHarris
Copy link
Collaborator Author

Closing without merge...

@BurtHarris BurtHarris closed this Oct 3, 2016
@BurtHarris BurtHarris deleted the experiment branch October 3, 2016 18:31
BurtHarris added a commit that referenced this pull request Apr 10, 2020
Merge changes from tunnelvisionlabs/antlr4ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants