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

User-friendly AST #136

Closed
dtolnay opened this issue May 20, 2017 · 2 comments
Closed

User-friendly AST #136

dtolnay opened this issue May 20, 2017 · 2 comments

Comments

@dtolnay
Copy link
Owner

dtolnay commented May 20, 2017

Compare the AST from strata-rust and the one from syn:

pub enum Pattern {
    Character(PatternCharacter),
    Ident(PatternIdent),
    Number(PatternNumber),
    Range(PatternRange),
    Reference(PatternReference),
    String(PatternString),
    Struct(PatternStruct),
    Tuple(PatternTuple),
}
pub enum Pat {
    Wild,
    Ident(BindingMode, Ident, Option<Box<Pat>>),
    Struct(Path, Vec<FieldPat>, bool),
    TupleStruct(Path, Vec<Pat>, Option<usize>),
    Path(Option<QSelf>, Path),
    Tuple(Vec<Pat>, Option<usize>),
    Box(Box<Pat>),
    Ref(Box<Pat>, Mutability),
    Lit(Box<Expr>),
    Range(Box<Expr>, Box<Expr>),
    Slice(Vec<Pat>, Option<Box<Pat>>, Vec<Pat>),
    Mac(Mac),
}

One of these is more consistent and more predictable than the other.

alexcrichton added a commit to alexcrichton/syn that referenced this issue May 23, 2017
This commit is a relatively large rewrite of the AST that `syn` exposes. The
main change is to expose enums-of-structs rather than
enums-with-huge-tuple-variants. The best example of this is `ItemKind::Fn` which
changed from:

    enum ItemKind {
        Fn(Box<FnDecl>, Unsafety, Constness, Option<Abi>, Generics, Box<Block>),
        ...
    }

to

    enum ItemKind {
        Fn(ItemFn),
        ...
    }

    struct ItemFn {
        decl: Box<FnDecl>,
        unsafety: Unsafety,
        constness: Constness,
        abi: Option<Abi>,
        generics: Generics,
        block: Box<Block>,
    }

This change serves a few purposes:

* It's now much easier to add fields to each variant of the ast, ast struct
  fields tend to be "by default ignored" in most contexts.
* It's much easier to document what each field is, as each field can have
  dedicated documentation.
* There's now canonicalized names for each field (the name of the field) which
  can help match `match` statements more consistent across a codebase.

A downside of this representation is that it can be a little more verbose to
work with in `match` statements and during constructions. Overall though I'd
feel at least that the readability improved significantly despite the extra
words required to do various operations.

Closes dtolnay#136
alexcrichton added a commit to alexcrichton/syn that referenced this issue May 23, 2017
This commit is a relatively large rewrite of the AST that `syn` exposes. The
main change is to expose enums-of-structs rather than
enums-with-huge-tuple-variants. The best example of this is `ItemKind::Fn` which
changed from:

    enum ItemKind {
        Fn(Box<FnDecl>, Unsafety, Constness, Option<Abi>, Generics, Box<Block>),
        ...
    }

to

    enum ItemKind {
        Fn(ItemFn),
        ...
    }

    struct ItemFn {
        decl: Box<FnDecl>,
        unsafety: Unsafety,
        constness: Constness,
        abi: Option<Abi>,
        generics: Generics,
        block: Box<Block>,
    }

This change serves a few purposes:

* It's now much easier to add fields to each variant of the ast, ast struct
  fields tend to be "by default ignored" in most contexts.
* It's much easier to document what each field is, as each field can have
  dedicated documentation.
* There's now canonicalized names for each field (the name of the field) which
  can help match `match` statements more consistent across a codebase.

A downside of this representation is that it can be a little more verbose to
work with in `match` statements and during constructions. Overall though I'd
feel at least that the readability improved significantly despite the extra
words required to do various operations.

Closes dtolnay#136
alexcrichton added a commit to alexcrichton/syn that referenced this issue May 23, 2017
This commit is a relatively large rewrite of the AST that `syn` exposes. The
main change is to expose enums-of-structs rather than
enums-with-huge-tuple-variants. The best example of this is `ItemKind::Fn` which
changed from:

    enum ItemKind {
        Fn(Box<FnDecl>, Unsafety, Constness, Option<Abi>, Generics, Box<Block>),
        ...
    }

to

    enum ItemKind {
        Fn(ItemFn),
        ...
    }

    struct ItemFn {
        decl: Box<FnDecl>,
        unsafety: Unsafety,
        constness: Constness,
        abi: Option<Abi>,
        generics: Generics,
        block: Box<Block>,
    }

This change serves a few purposes:

* It's now much easier to add fields to each variant of the ast, ast struct
  fields tend to be "by default ignored" in most contexts.
* It's much easier to document what each field is, as each field can have
  dedicated documentation.
* There's now canonicalized names for each field (the name of the field) which
  can help match `match` statements more consistent across a codebase.

A downside of this representation is that it can be a little more verbose to
work with in `match` statements and during constructions. Overall though I'd
feel at least that the readability improved significantly despite the extra
words required to do various operations.

Closes dtolnay#136
alexcrichton added a commit to alexcrichton/syn that referenced this issue May 23, 2017
This commit is a relatively large rewrite of the AST that `syn` exposes. The
main change is to expose enums-of-structs rather than
enums-with-huge-tuple-variants. The best example of this is `ItemKind::Fn` which
changed from:

    enum ItemKind {
        Fn(Box<FnDecl>, Unsafety, Constness, Option<Abi>, Generics, Box<Block>),
        ...
    }

to

    enum ItemKind {
        Fn(ItemFn),
        ...
    }

    struct ItemFn {
        decl: Box<FnDecl>,
        unsafety: Unsafety,
        constness: Constness,
        abi: Option<Abi>,
        generics: Generics,
        block: Box<Block>,
    }

This change serves a few purposes:

* It's now much easier to add fields to each variant of the ast, ast struct
  fields tend to be "by default ignored" in most contexts.
* It's much easier to document what each field is, as each field can have
  dedicated documentation.
* There's now canonicalized names for each field (the name of the field) which
  can help match `match` statements more consistent across a codebase.

A downside of this representation is that it can be a little more verbose to
work with in `match` statements and during constructions. Overall though I'd
feel at least that the readability improved significantly despite the extra
words required to do various operations.

Closes dtolnay#136
@dtolnay dtolnay reopened this May 23, 2017
@dtolnay
Copy link
Owner Author

dtolnay commented May 23, 2017

Reopening until I get a chance to look through all the naming and see if fuzzy-pickles has any better names for things.

@dtolnay
Copy link
Owner Author

dtolnay commented Nov 13, 2017

This is an ongoing effort but we are in a much better spot now. The new patterns even enable some interesting improvements in functionality, such as #218.

@dtolnay dtolnay closed this as completed Nov 13, 2017
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

No branches or pull requests

1 participant