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.

Sign up for GitHub

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

Jump to bottom

Improve AST node initialization #469

Merged
merged 6 commits into from
May 17, 2022
Merged

Improve AST node initialization #469

merged 6 commits into from
May 17, 2022

Conversation

msujew
Copy link
Member

@msujew msujew commented Apr 7, 2022 •
edited
Loading

Solves three issues:

  1. Adds a default value for boolean properties
  2. Improves handling of array properties, which are now only initialized when needed.
  3. Introduces an AST reflection interpreter

@msujew msujew added the ast AST structure related issue label Apr 7, 2022
@msujew msujew marked this pull request as draft April 7, 2022 15:00
@msujew msujew marked this pull request as ready for review April 7, 2022 15:22
@spoenemann
Copy link
Contributor

Introduces an AST reflection interpreter

This is about #421 (comment) ?

@msujew
Copy link
Member Author

msujew commented Apr 11, 2022

This is about #421 (comment) ?

@spoenemann I had that in mind, but I added the ASTReflection interpreter for another reason, since it's actually used inside of the parser now, so every parser test needs an ASTReflection now. I could've used a stub, but we needed the interpreter anyway, so I wrote that as well.

dhuebner
dhuebner approved these changes Apr 20, 2022
View reviewed changes
packages/langium/src/grammar/ast-reflection-interpreter.ts
const allTypes = collectedTypes.interfaces.map(e => e.name).concat(collectedTypes.unions.map(e => e.name));
const references = buildReferenceTypes(collectedTypes);
const metaData = buildTypeMetaData(collectedTypes);
const superTypeMap = buildSupertypeMap(collectedTypes);
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: How are sub types handled in ast reflection?
For interface A extends B, C {} will there be an entry for A with all inherited mandatory entries from B and C?

@msujew msujew force-pushed the msujew/improve-ast-node-init branch from 9a70b19 to 11e5bb5 Compare April 22, 2022 22:33
spoenemann
spoenemann approved these changes May 10, 2022
View reviewed changes
Copy link
Contributor

@spoenemann spoenemann left a comment

Choose a reason for hiding this comment

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

Great! 👍

packages/langium/src/grammar/ast-reflection-interpreter.ts Outdated Show resolved Hide resolved
packages/langium/src/grammar/type-system/types-util.ts Outdated Show resolved Hide resolved
packages/langium/src/syntax-tree.ts Outdated Show resolved Hide resolved
@msujew msujew force-pushed the msujew/improve-ast-node-init branch from 8af1128 to 324686b Compare May 17, 2022 13:18
@msujew msujew merged commit 2392ddd into main May 17, 2022
@msujew msujew deleted the msujew/improve-ast-node-init branch May 17, 2022 13:20
@msujew msujew mentioned this pull request May 19, 2022
Merged
@montymxb montymxb mentioned this pull request May 20, 2022
Closed
@spoenemann spoenemann added this to the v0.4.0 milestone Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhuebner dhuebner dhuebner approved these changes

@spoenemann spoenemann spoenemann approved these changes

Assignees
No one assigned
Labels
ast AST structure related issue
Projects
None yet
Milestone
v0.4.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@msujew @spoenemann @dhuebner