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

Add attribute mixedAST to simplify mixed AST+PSI creation #316

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vlad20012
Copy link
Contributor

@vlad20012 vlad20012 commented Jan 14, 2023

We are used to two different kinds of PSI implementations:

  1. ASTDelegatePsiElement
+-----+         +-----+
| PSI | ------> | AST |
+-----+         +-----+
  1. StubBasedPsiElementBase
+-----+         +-----+
| PSI | ------> | AST |
+-----+ \ OR    +-----+
         \
          \     +------+
           \ -> | Stub |
                +------+

But really there is a third option - mix PSI and AST in a single class/object:

+---------+
| PSI/AST |
+---------+

This is useful when a PSI element does not have a stub. In this case, additional AST object is not allocated, so we win in memory consumption, and the tree traversing will be fasted, so we win in CPU as well.
A PSI element can be mixed if it implements both ASTNode and PsiElement interfaces. This is already done in the platform class CompositePsiElement.

When extending CompositePsiElement, a psi implementation slightly differs from the one extending ASTDelegatePsiElement. In particular, token accessors use findPsiChildByType instead of findChildByType and different constructor signature is needed:

public Element1Impl(IElementType type) {
  super(type);
}

Grammar-Kit supports mixed AST at the moment, but currently it figures out whether a BNF rule uses mixedAST or not by reading .class file content of its extends attribute and comparing its chain of superclasses with com.intellij.psi.impl.source.tree.CompositePsiElement name (1, 2).

Such approach requires extracting all possible superclasses and mixins of PSI nodes to a separate gradle module in order to compile it prior to Grammar-Kit invocation, which is a quite complex setup.

In this PR I've simplified it by introducing mixedAST attribute. If it is set to true (and the rule does not have a stub), Grammar-Kit will use mixedAST style for generated PSI impls without trying to read any .class files content at runtime. The attribute can be applied to a specific rule or globally (so a rule will be treated as mixedAST unless it has a stub)

@lerno
Copy link

lerno commented May 2, 2023

I am looking at solutions to the Grammar-Kit gradle plugin problem and stumbled over this pull request. It seems interesting for other reasons, but mostly for the codegen .class reading issues. I'd love if the GrammarKit team had a look at this and either rejected or accepted it.

@ice1000
Copy link

ice1000 commented May 14, 2024

What is the status of this PR? Is it being considered?

@gregsh
Copy link
Collaborator

gregsh commented May 14, 2024

Current status:

A year ago I and @vlad20012 discussed that a separate mixedAST flag is not needed.

If possible, fast logic must always be used. generate.exact-types attribute can be used to detect the case.

@ice1000
Copy link

ice1000 commented May 15, 2024

Where can I learn more about generate.exact-types?

@ice1000
Copy link

ice1000 commented May 15, 2024

Did you mean #317? It's also open, hmm

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.

4 participants