Split libsyntax apart

[Centril]

In this PR the general idea is to separate the AST, parser, and friends by a more data / logic structure (tho not fully realized!) by separating out the parser and macro expansion code from libsyntax. Specifically have now three crates instead of one (libsyntax):

• libsyntax:

  • concrete syntax tree (syntax::ast)

  • definition of tokens and token-streams (syntax::{token, tokenstream}) -- used by syntax::ast

  • visitors (syntax::visit, syntax::mut_visit)

  • shared definitions between libsyntax_expand

  • feature gating (syntax::feature_gate) -- we could possibly move this out to its own crater later.

  • attribute and meta item utilities, including used-marking (syntax::attr)

  • pretty printer (syntax::print) -- this should possibly be moved out later. For now I've reduced down the dependencies to a single essential one which could be broken via ParseSess. This entails that e.g. Debug impls for Path cannot reference the pretty printer.

  • definition of ParseSess (syntax::sess) -- this is used by syntax::{attr, print, feature_gate} and is a common definition used by the parser and other things like librustc.

  • the syntax::source_map -- this includes definitions used by syntax::ast and other things but could ostensibly be moved syntax_pos since that is more related to this module.

  • a smattering of misc utilities not sufficiently important to itemize -- some of these could be moved to where they are used (often a single place) but I wanted to limit the scope of this PR.

• librustc_parse:

  • parser (rustc_parse::parser) -- reading a file and such are defined in the crate root tho.

  • lexer (rustc_parse::lexer)

  • validation of meta grammar (post-expansion) in (rustc_parse::validate_attr)

• libsyntax_expand -- this defines the infra for macro expansion and conditional compilation but this is not libsyntax_ext; we might want to merge them later but currently libsyntax_expand is depended on by librustc_metadata which libsyntax_ext is not.

  • conditional compilation (syntax_expand::config) -- moved from syntax::config to here

  • the bulk of this crate is made up of the old syntax::ext

r? @estebank

[petrochenkov]

👍 on the general direction.
Some comments from a quick review:

• The PR is already too big, please try to wrap it up without adding extra changes, unless they are necessary for splitting the crates.
• The previous point also applies to local changes (like "simplify maybe_stage_features"), which are not necessary for splitting the crates and can be done later in a separate PR.
• Please avoid tiny modules (like libsyntax_ext/util.rs, libsyntax/util/classify.rs), their contents can be moved into their parent modules.
• syntax_macros -> syntax_expand, the former looks like it contains macros used in libsyntax.
• syntax::ext -> syntax::expand, merge its tiny nested modules like base.rs and proc_macro.rs into it.
• Style: please no multiline imports, a use item should start and end on the same line.
• Moving attribute validation from PostExpansionVisitor::visit_attr to AST validation is incorrect, because this validation is done for inert attributes that have to disappear during macro expansion, like attributes on macro calls. See how PostExpansionVisitor::visit_attr is used from feature_gate::check::check_attribute which is called from expand.rs.
• Please try to replace the FFI hacks connecting crates A and B with traits defined by their common dependencies. I'm not sure that the necessary crates are always linked together.

[petrochenkov]

Regarding use of cfg-expansion from parse_item_mod - see #64197.

[Centril]

Some comments from a quick review:

Thanks for that :)

The PR is already too big, please try to wrap it up without adding extra changes, unless they are necessary for splitting the crates.

Yeah my goal with this PR was to propose a general direction and then split out smaller individual commits to merge first, especially for some of the larger or drive-by changes (like the one you referenced).

Please avoid tiny modules (like libsyntax_ext/util.rs, libsyntax/util/classify.rs), their contents can be moved into their parent modules.

Hmm... Some of these small modules were pre-existing so if possible I'd like to avoid more drive-by changes in this PR. :)

syntax_macros -> syntax_expand, the former looks like it contains macros used in libsyntax.

Nice! I'll use that. Better than my naming. :)

Style: please no multiline imports, a use item should start and end on the same line.

Will do.

Moving attribute validation from PostExpansionVisitor::visit_attr to AST validation is incorrect, because this validation is done for inert attributes that have to disappear during macro expansion, like attributes on macro calls. See how PostExpansionVisitor::visit_attr is used from feature_gate::check::check_attribute which is called from expand.rs.

Hmm... I inserted a call to validate_attr::check_meta(...); in expand.rs specifically to deal with this. Shouldn't that be sufficient? It feels very wrong to do validation logic in feature gating rather than a validation pass, so if AST validation is not good then I think we should add another pass just after feature gating. This is also a good idea because it doesn't create a cyclic dependency between the parser and feature gating.

Please try to replace the FFI hacks connecting crates A and B with traits defined by their common dependencies. I'm not sure that the necessary crates are always linked together.

By connecting them via e.g. ParseSess (in the parser) or Session (in lowering)? Sure, my initial idea was to do that using just function pointers (dyn Fn(...) -> ...) is unnecessary because these do not require state) but @Mark-Simulacrum suggested that this FFI hack is commonly used in C++-land for ThinLTO purposes but I guess there's not going to be notable perf differences either way from one indirect call here?

[Mark-Simulacrum]

I only theorized about the FFI hack. I think traits are better, generally speaking, and using &dyn Trait -- maybe even &'static dyn Trait since we always go to a zero-sized struct implementing the trait or so makes a lot of sense.

[Centril]

@petrochenkov Rebased & squashed.

[petrochenkov]

I never thought about the syntax vs rustc distinction, but now when eddyb mentioned it, I agree that it would be reasonable to migrate things to rustc.
Since this PR introduces syntax_parse, it probably makes sense to immediately introduce it as rustc_parse, crates introduced in the previous PRs can be renamed later (syntax_expand -> rustc_expand, syntax_ext -> rustc_builtin_macros, syntax/syntax_pos - ???).

[Centril]

Since this PR introduces syntax_parse, it probably makes sense to immediately introduce it as rustc_parse, crates introduced in the previous PRs can be renamed later (syntax_expand -> rustc_expand, syntax_ext -> rustc_builtin_macros, syntax/syntax_pos - ???).

Done. I would propose syntax -> rustc_ast and syntax_pos -> rustc_pos

[petrochenkov]

@bors r+

[bors]

📌 Commit 4ae2728 has been approved by petrochenkov

[petrochenkov]

@Centril

I would propose syntax -> rustc_ast and syntax_pos -> rustc_pos

rustc_pos -> rustc_span perhaps?

The crate consists of:

• source map - kind of position, kind of span, spans and positions point into the source map
• span - kind of position, literally span
• hygiene - not a position, part of a span
• edition - part of hygiene - not a position, part of a span
• symbol/ident - neither position, nor span (ident has a span though)

[Centril]

rustc_pos -> rustc_span perhaps?

👍

[bors]

⌛ Testing commit 4ae2728 with merge a3b6e57...

[bors]

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing a3b6e57 to master...