Stream: t-compiler/rust-analyzer

Topic: `tt::TokenTree` to `ast::TokenTree`


Jonas Schievink [he/him] (Dec 18 2020 at 14:58, on Zulip):

I need to go from the types in the tt crate, back to CST nodes, in order to reparse an attribute. Is there any preexisting way to do so? If not, what would be the best place to put this functionality? Do we even want to allow this?

I've seen ast::Path::parse etc., but those only work with macro fragments.

matklad (Dec 18 2020 at 14:59, on Zulip):

Hm....

matklad (Dec 18 2020 at 14:59, on Zulip):

My gut feeling is that we should do exactly the opposite

matklad (Dec 18 2020 at 14:59, on Zulip):

Attributes should be parasable out of tts

matklad (Dec 18 2020 at 14:59, on Zulip):

(I think they actually are?)

Jonas Schievink [he/him] (Dec 18 2020 at 15:00, on Zulip):

I could make them parseable from tt types, I think

Jonas Schievink [he/him] (Dec 18 2020 at 15:00, on Zulip):

They only contain tt types, at least, so it should be possible

matklad (Dec 18 2020 at 15:01, on Zulip):

Could you tell why you need that?

matklad (Dec 18 2020 at 15:01, on Zulip):

Is this cfg_attr?

Jonas Schievink [he/him] (Dec 18 2020 at 15:01, on Zulip):

Yeah

Jonas Schievink [he/him] (Dec 18 2020 at 15:02, on Zulip):

I'm trying to avoid having two parsers for this syntax, so I wanted to reuse the existing one

matklad (Dec 18 2020 at 15:03, on Zulip):

Uhu...

matklad (Dec 18 2020 at 15:03, on Zulip):

Hm, otoh, making a fragment for that would also work

Jonas Schievink [he/him] (Dec 18 2020 at 15:12, on Zulip):

The make API also sometimes provides raw parsers, maybe it should be added there? Not entirely sure what the difference between that and the parse functions is, the latter look macro-specific

Jonas Schievink [he/him] (Dec 18 2020 at 15:13, on Zulip):

Looks like this is not going to work with None delimiters in any case

matklad (Dec 18 2020 at 15:25, on Zulip):

@Jonas Schievink all raw parsers in make should be private

Jonas Schievink [he/him] (Dec 18 2020 at 15:25, on Zulip):

Currently there's these:

pub fn name(text: &str) -> ast::Name {
    ast_from_text(&format!("mod {};", text))
}

pub fn name_ref(text: &str) -> ast::NameRef {
    ast_from_text(&format!("fn f() {{ {}; }}", text))
}

pub fn ty(text: &str) -> ast::Type {
    ast_from_text(&format!("impl {} for D {{}};", text))
}
matklad (Dec 18 2020 at 15:29, on Zulip):

The last one is a bug

matklad (Dec 18 2020 at 15:29, on Zulip):

The first two are not really parsers -- they produce a single indivisible node

matklad (Dec 18 2020 at 15:31, on Zulip):

I guess, @matklad needs to document the invariants of make module somewhere

Jonas Schievink [he/him] (Dec 18 2020 at 15:32, on Zulip):

Ah, right

matklad (Dec 18 2020 at 15:32, on Zulip):

Also, urgh, I wish there was some way to enforce "this shouldn't happen" properties in code...

matklad (Dec 18 2020 at 15:33, on Zulip):

keeping raw text methods of make priviate is a constant struggle

matklad (Dec 18 2020 at 15:33, on Zulip):

See also comment on Name::new_text

Jonas Schievink [he/him] (Dec 18 2020 at 15:34, on Zulip):

yeah I did see that one

Jonas Schievink [he/him] (Dec 18 2020 at 15:34, on Zulip):

but make::name("bla").as_name() sidesteps that

matklad (Dec 18 2020 at 15:36, on Zulip):

Not really, AsName shouldn't exported to the IDE

matklad (Dec 18 2020 at 15:36, on Zulip):

(I think...)

matklad (Dec 18 2020 at 15:37, on Zulip):

(but also enforcing AsName should be inaccessible in crates/ide is hard)

Jonas Schievink [he/him] (Dec 18 2020 at 15:38, on Zulip):

Right, so we don't expose text parsers in make to prevent the IDE from parsing raw strings, not the compiler crates?

matklad (Dec 18 2020 at 15:38, on Zulip):

Not really

matklad (Dec 18 2020 at 15:38, on Zulip):

The primary goal here is that we want to switch to a fully-typed API in the future

Jonas Schievink [he/him] (Dec 18 2020 at 15:38, on Zulip):

matklad said:

(but also enforcing AsName should be inaccessible in crates/ide is hard)

I feel like adding a comment that explains this would already go a long way

matklad (Dec 18 2020 at 15:39, on Zulip):

Where we just assemble AST nodes from smaller AST nodes

matklad (Dec 18 2020 at 15:39, on Zulip):

We don't do this today because that's a whole lot of code to write by hand, and parse . format! is just shorter

matklad (Dec 18 2020 at 15:40, on Zulip):

THere' WIP PR by @Lukas Wirth which uses ungrammar for that.

matklad (Dec 18 2020 at 15:40, on Zulip):

if we expose raw text API, then refactoring later will be more painful. And we'll add a bunch of stringly-typed code patterns, which I think we should try to avoid.

Jonas Schievink [he/him] (Dec 18 2020 at 16:35, on Zulip):

So it seems like the ast::Path::parse et al should also be removed, since it parses a raw &str, right?

matklad (Dec 18 2020 at 16:38, on Zulip):

Well, not really

matklad (Dec 18 2020 at 16:38, on Zulip):

Parsing a Path from string is a valid operation to ahve

matklad (Dec 18 2020 at 16:38, on Zulip):

(and, for example, intra-doc links fundamentally require it)

matklad (Dec 18 2020 at 16:39, on Zulip):

It's just the connection of making new AST nodes and parsing is a hack which shouldn't be exposed

Jonas Schievink [he/him] (Dec 18 2020 at 16:39, on Zulip):

Hmm, okay, not sure I understand

matklad (Dec 18 2020 at 16:41, on Zulip):

So imaging a make API which allows only creating tokens from strings

matklad (Dec 18 2020 at 16:41, on Zulip):

and all composite nodes require constituent elements

matklad (Dec 18 2020 at 16:41, on Zulip):

SO, to create (A, B), you write make::tuple_type(vec![a, b])

matklad (Dec 18 2020 at 16:42, on Zulip):

and that internallly assembles a tree using existing trees

Jonas Schievink [he/him] (Dec 18 2020 at 16:42, on Zulip):

Yeah, that does makes sense to me

matklad (Dec 18 2020 at 16:42, on Zulip):

In that world, the operation parsre this bit of text as type isn't really expressible and necessary. If you want to create a type, you create it from smaller types

Jonas Schievink [he/him] (Dec 18 2020 at 16:43, on Zulip):

I just wonder why going from &str to ast::Path is fine, since that's clearly not a token

matklad (Dec 18 2020 at 16:43, on Zulip):

Becausee that's parsing and not node creation

matklad (Dec 18 2020 at 16:44, on Zulip):

Hm, where Path::parse is actually defined?

matklad (Dec 18 2020 at 16:44, on Zulip):

It shoudl allow to return an error

matklad (Dec 18 2020 at 16:44, on Zulip):

while all make functions are infailable.

Jonas Schievink [he/him] (Dec 18 2020 at 16:44, on Zulip):

Ah, okay, so it needs to return a Result

Jonas Schievink [he/him] (Dec 18 2020 at 16:44, on Zulip):

It does, I mean

Jonas Schievink [he/him] (Dec 18 2020 at 16:44, on Zulip):

Or a ParseResult would also work, I suppose

matklad (Dec 18 2020 at 16:44, on Zulip):

Ah, here it is

matklad (Dec 18 2020 at 16:44, on Zulip):

pub fn parse(text: &str) -> Result<Self, ()> {

matklad (Dec 18 2020 at 16:45, on Zulip):

Yeah, it needs some way to signal an error

matklad (Dec 18 2020 at 16:45, on Zulip):

This is one of the "won't fix " bugs today

Jonas Schievink [he/him] (Dec 18 2020 at 16:45, on Zulip):

Alright, yeah, that does make sense

matklad (Dec 18 2020 at 16:45, on Zulip):

Sometimes nodes are syntactically invalid. If you use invalid type to make, say, a let x: ty = todo!() expressions, and you go via format! in the middle

matklad (Dec 18 2020 at 16:46, on Zulip):

you might not parse let x: as a proper let statement

matklad (Dec 18 2020 at 16:46, on Zulip):

Node base editing should fix that problem

Jonas Schievink [he/him] (Dec 18 2020 at 16:47, on Zulip):

So, that sounds like we should replace calls to make::ty with ast::Type::parse then, right?

matklad (Dec 18 2020 at 16:48, on Zulip):

Not exactly

matklad (Dec 18 2020 at 16:48, on Zulip):

rather, with make::ty_unit, make::ty_path, etc

matklad (Dec 18 2020 at 16:49, on Zulip):

let me show a quick pr...

matklad (Dec 18 2020 at 16:50, on Zulip):

https://github.com/rust-analyzer/rust-analyzer/pull/6931

matklad (Dec 18 2020 at 16:50, on Zulip):

Something like this

matklad (Dec 18 2020 at 16:50, on Zulip):

Basically, strongly-type the thing

matklad (Dec 18 2020 at 16:50, on Zulip):

rather than stringly-type the thing

Jonas Schievink [he/him] (Dec 18 2020 at 16:50, on Zulip):

yeah, that makes sense when possible

Jonas Schievink [he/him] (Dec 18 2020 at 16:51, on Zulip):

but this use here goes from Type to ast::Type https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/assists/src/ast_transform.rs#L109

matklad (Dec 18 2020 at 16:52, on Zulip):

yeah...

Jonas Schievink [he/him] (Dec 18 2020 at 16:52, on Zulip):

I suppose we could make that a method on Type

Jonas Schievink [he/him] (Dec 18 2020 at 16:52, on Zulip):

or something

matklad (Dec 18 2020 at 16:52, on Zulip):

Looking at that now, and urgh.

matklad (Dec 18 2020 at 16:52, on Zulip):

I think dispaly source code should return a SyntaxNode / ast::Something

matklad (Dec 18 2020 at 16:52, on Zulip):

probably

matklad (Dec 18 2020 at 16:52, on Zulip):

HirDispaly:: fn display_source_code<'a> is the culprit here

Jonas Schievink [he/him] (Dec 18 2020 at 16:53, on Zulip):

ah, yeah

matklad (Dec 18 2020 at 16:57, on Zulip):

Well, I think it's fine to call parse inside display_source_code :D

matklad (Dec 18 2020 at 16:57, on Zulip):

and plan to think about this propertly later

matklad (Dec 18 2020 at 16:58, on Zulip):

but yeah, I think that we probably should describe hir rendering in terms of syntax trees. This shoudl be more composable with complex refactors

matklad (Dec 23 2020 at 10:03, on Zulip):

@Jonas Schievink submitted https://github.com/rust-analyzer/rust-analyzer/pull/7017 with doc clarifications

Last update: Jul 28 2021 at 05:15UTC