Stream: t-compiler/wg-rls-2.0

Topic: indeterministic proc-macro


Edwin Cheng (May 06 2020 at 08:26, on Zulip):

I have read some articles written by @matklad that some proc-macro are impossible to infer and analysis. But I am tracing a bug in some proc-macros that is indeterministic: i.e. It expands to different syntax orders every time( maybe using Hashmap internally)

matklad (May 06 2020 at 08:27, on Zulip):

Hm, yeah, that would be bad for salsa...

matklad (May 06 2020 at 08:27, on Zulip):

Dear, we do need to wasm up all proc macros...

Florian Diebold (May 06 2020 at 08:29, on Zulip):

(deleted)

Florian Diebold (May 06 2020 at 08:31, on Zulip):

but I guess as long as the binding files don't change, that's maybe not the worst

matklad (May 06 2020 at 08:32, on Zulip):

Yea, I've already left a "please, don't" comment on their repo: https://github.com/microsoft/winrt-rs/issues/107#issuecomment-622402448 :D

Edwin Cheng (May 06 2020 at 08:36, on Zulip):

I know they are starting to discuss about 2021 edition features, you might have to persuade to them hardly.

Edwin Cheng (May 06 2020 at 08:37, on Zulip):

FYI, the actual panic is look like this :

thread '<unnamed>' panicked at 'can't resolve local ptr to SyntaxNode: SyntaxNodePtr { range: 160..329, kind: IMPL_DEF }', crates\ra_syntax\src\ptr.rs:30:28
matklad (May 06 2020 at 08:37, on Zulip):

I don't think it's a good idea to target any lang-changes for 2021 edition -- not enough time

matklad (May 06 2020 at 08:37, on Zulip):

for 2024, I hope we'll be able to clean up the lang with an eye towards IDE

Edwin Cheng (May 06 2020 at 08:39, on Zulip):

Okay, then we have to hotfix this bug now :(
I propose we store a cache for expansion result, and return its for second call for expand_proc_macro

matklad (May 06 2020 at 08:39, on Zulip):

Maybe we should move proc-macros to a separate table, which is not LRUd?

Edwin Cheng (May 06 2020 at 08:42, on Zulip):

Thats much better of course.

But would you mind give me some pointer about that part of doc in salsa ? I know very basic usage about it.

matklad (May 06 2020 at 08:44, on Zulip):

Seems like we have zero docs about this D:

matklad (May 06 2020 at 08:46, on Zulip):

wait, where do we actually set up LRU? Looks like I forgot how it works

matklad (May 06 2020 at 08:47, on Zulip):

Ah, it's well hidden. @Edwin Cheng take a look at RootDatabase::update_lru_capacity method

matklad (May 06 2020 at 08:47, on Zulip):

It calls self.query_mut(ra_db::ParseQuery).set_lru_capacity(lru_capacity);

matklad (May 06 2020 at 08:47, on Zulip):

What this does, in terms of salsa, is ensures that at any time there's at most lru_capacity values in a given table

matklad (May 06 2020 at 08:48, on Zulip):

and values are evicted on LRU bases

matklad (May 06 2020 at 08:48, on Zulip):

I know very basic usage about it.

I think you might want to spend some time reading the source code for salsa -- it's a very interesting and well-writen piece of technology :)

Edwin Cheng (May 06 2020 at 08:55, on Zulip):

Will do! Thanks

pksunkara (May 06 2020 at 08:59, on Zulip):

I agree. I had fun reading up on rust-analyzer salsa stuff :D

Christopher Durham (May 06 2020 at 16:52, on Zulip):

@matklad just to scare you more, pest currently also reads from another file in its derive macro (though we do offer an inline option now).

Christopher Durham (May 06 2020 at 16:53, on Zulip):

I'm slowly pushing for a proc_macro API for loading a file (with the Rust lexer and getting Span information) which should be less problematic than just reading an arbitrary file, though.

Christopher Durham (May 06 2020 at 16:53, on Zulip):

(because it tells the macro driver to do the file loading)

Christopher Durham (May 06 2020 at 16:54, on Zulip):

Basically, fn proc_macro::open(&Path) -> TokenStream.

Florian Diebold (May 06 2020 at 16:55, on Zulip):

if it returns a TokenStream, it can only be used for files that can be lexed as Rust though, right?

Christopher Durham (May 06 2020 at 16:56, on Zulip):

Yes, but I think that's the majority of cases

Christopher Durham (May 06 2020 at 16:56, on Zulip):

And I'm also in favor of making TokenStream able to represent invalid tokens

Christopher Durham (May 06 2020 at 16:57, on Zulip):

(by e.g. making "2.0" meta::tokens::TokenStream closer to rustc_lexer than proc_macro::TokenStream would be one "simple" way of doing so)

pksunkara (May 07 2020 at 09:05, on Zulip):

I have a proc_macro that processes a lot of files using regex (not TokenStream) and builds a TokenStream. I understand that this is bad practice, but I couldn't achieve this any other way. What exactly do we need to support this in the IDE? Maybe some kind of flag to denote a proc_macro as indeterministic? That would allow the proc_macro server to run it everytime.

Edwin Cheng (May 07 2020 at 09:32, on Zulip):

I can imagine the wasm proc-macro should be no-std with some explict api for IO and other stuffs. In theory we could hook the IO api in wasm VM and detect if anything is changed in expansion and invalidate all related caches.

But well, it is quite complicated ...

Christopher Durham (May 07 2020 at 21:28, on Zulip):

A small step would just to be allowing proc macros to use cargo:rerun-if-changed like build scripts do.

Last update: May 29 2020 at 17:30UTC