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

[V1] Add source location and spans to V1 AstNodes #1608

Open
RCHowell opened this issue Oct 4, 2024 · 1 comment
Open

[V1] Add source location and spans to V1 AstNodes #1608

RCHowell opened this issue Oct 4, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@RCHowell
Copy link
Member

RCHowell commented Oct 4, 2024

public abstract class AstNode {

    @NotNull
    public final Span span;

    // .........
}

public class Location {
    public final int line;
    public final int column;
}

public class Span {
    public final Location start;
    public final Location end;
}

Something like https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql/parser/SqlParserPos.java

But it would make sense to have these be lazy so that we don't create many needless objects until the locations are actually used .. just to cut down on object creation for ASTs. Really, a node should only need its line/column pair and can compute a span from the children.

@alancai98
Copy link
Member

Alternative modeling could follow what we do in partiql-lang-rust. That is, create a parser result which stores the AST along with a source location map https://github.com/partiql/partiql-lang-rust/blob/3f9d17fb38edafb23c1908af27b06c78d66680cd/partiql-parser/src/lib.rs#L81-L86.

Modeling with a map should offer the same functionality as spans on every node and not require additional fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants