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

Refactor to support tracking undeclared functions and types #11

Merged
merged 8 commits into from
Jul 22, 2023

Conversation

AndrewRayCode
Copy link
Collaborator

@AndrewRayCode AndrewRayCode commented Jun 22, 2023

This is a significant set of changes, some breaking.

The main goal of this change is to support tracking whether or not functions and types have declarations in the scope. The type and function scope entries now have a definition key, which points to the definition of a function or type.

Features:

  • The parser now supports overloaded function tracking in scope. A function scope index went from { [fnName]: { references: AstNode[] } to { [fnName]: { [overloadSignature]: { declaration?: AstNode, references: AstNode[], ... } } }. This is a breaking change. Note that if you're using the renameFunctions utility function provided by the parser, this change may be opaque to you.
  • The semantic analysis of this library is still mostly non-existent, but there are now improved warnings for missing function and type definitions
  • New failOnWarn parser option flag to raise errors on things like undefined variables.

Breaking API changes:

  • Adds a new TypeNameNode AST node type, to distinguish a type name from an identifier in the AST. If you're using node visitors to visit identifier nodes, you'll need a new visitor for type_name nodes.
  • Removes ParameterDeclaratorNode and moves everything into ParameterDeclarationNode
  • In the AST node Typescript definitions, any time I didn't know what node was, I put in any. I replaced that with AstNode. I don't yet know if I want to keep this, because AstNode could lead to more issues than it causes. It could lead to type errors and forced casting that wouldn't come along with any. Like it might force you to make sure our node isn't a LiteralNode even though technically the grammar doesn't allow for that.
  • Previously, a scope binding (aka a variable declaration use), had the type { initializer: declaration_ast_node, references: [ast_node, ...] }, where the ast_node could be the declaration node containing the identifier. It turns out initializer was never part of the Typescript type, so you might never have seen it. Either way, initializer is renamed to declaration, and it now points to the identifier node rather than the declaration.

Internal development:

  • All of the functions that were defined in src/parser/glsl-grammar.pegjs are now rewritten in typescript and extracted into an external file.
  • Various clean-ups of the grammar, like removing the duplicate path function_prototype_no_new_scope
  • Cleanup of tsconfig.json file
  • Adds the tracer Peggyjs option to the parser, for debugging
  • Removes preprocessor tests from parse.ast.ts
  • Breaking out of source code into more logical files

@AndrewRayCode AndrewRayCode changed the title Refactor of helpers into typescript files Refactor to support tracking undeclared functions and types Jul 3, 2023
Copy link

@aanari aanari left a comment

Choose a reason for hiding this comment

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

LGTM

@AndrewRayCode AndrewRayCode removed the WIP label Jul 5, 2023
This is a significant set of changes, some breaking.

The main goal of this change is to support tracking whether or not functions and types have declarations in the scope. The type and function scope entries now have a `definition` key, which points to the definition of a function or type.

Features:

- The parser now supports overloaded function tracking in scope. A
  function scope index went from `{ [fnName]: { references: AstNode[] }`
  to  `{ [fnName]: { [overloadSignature]: { declaration?: AstNode,
  references: AstNode[], ... } } }`. This is a breaking change. Note
  that if you're using the `renameFunctions` utility function provided
  by the parser, this change _may_ be opaque to you.
- The semantic analysis of this library is still mostly non-existent,
  but there are now improved warnings for missing function and type
  definitions
- New `failOnWarn` parser option flag to raise errors on things like
  undefined variables.

Breaking API changes:
- Adds a new `TypeNameNode` AST node type, to distinguish a type name
  from an identifier in the AST. If you're using node visitors to visit
  `identifier` nodes, you'll need a new visitor for `type_name` nodes.
- Removes `ParameterDeclaratorNode` and moves everything into
  `ParameterDeclarationNode`
- In the AST node Typescript definitions, any time I didn't know what
  node was, I put in `any`. I replaced that with `AstNode`. I don't yet
  know if I want to keep this, because `AstNode` could lead to more
  issues than it causes. It could lead to type errors and forced casting
  that wouldn't come along with `any`. Like it might force you to make
  sure our node isn't a `LiteralNode` even though technically the
  grammar doesn't allow for that.

Internal development:
- All of the functions that were defined in
  `src/parser/glsl-grammar.pegjs` are now rewritten in typescript and
  extracted into an external file.
- Various clean-ups of the grammar, like removing the duplicate path
  `function_prototype_no_new_scope`
- Cleanup of tsconfig.json file
- Adds the `tracer` Peggyjs option to the parser, for debugging
- Removes preprocessor tests from parse.ast.ts
- Breaking out of source code into more logical files
@AndrewRayCode AndrewRayCode merged commit 1f51744 into main Jul 22, 2023
1 check passed
@AndrewRayCode AndrewRayCode deleted the typescript-scopes branch July 22, 2023 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants