Parse validation is oblivious to macro calls

[Veykril]

Rust has two kings of parse errors:

• Pre-expansion parse errors, error that occur while parsing when something unexpected crops up
• Post-expansion parse errors (validation errors), parses that are rejected post macro expansion

The latter allows for stuff like unsafe mod foo {} to be accepted by attributes without issues, as long as they don't emit unsafe mod again. That is, unsafe mod is rejected in post expansion by validation. rustc does this by walking the fully expanded file tree with a validation pass. Our validation pass right now walks the unexpanded trees though which means we would (if we had a validation diagnostic for unsafe mod) diagnose the following:

#[cfg(False)]
unsafe mod m {}

where as rustc does not. The reason for this is mainl that rust-analyzer does not work on the fully expanded parse tree of a file, we keep the file parse and the macro parses separate so we need to figure something out here.

[ChayimFriedman2]

So I thought about this a bit and I think we should walk the tree when requesting diagnostics. I will try implementing that and see how it looks.

An alternative is to not walk the tree, instead having validation errors returned by parse_or_expand() but only for nodes that are not inside macro calls. This is somewhat problematic with our setup because parse_or_expand() is defined in hir-expand, but to correctly identify whether something is inside macro we need hir-def.

Now thinking about it, a third alternative is to do these in the def map errors. I think I like this option more than the rest, but this has the downside that we're running the validation even when it's not really needed. Edit: Nevermind we can't do that the defmap can't touch the parse tree for incrementality reasons.

[Veykril]

An alternative is to not walk the tree, instead having validation errors returned by parse_or_expand() but only for nodes that are not inside macro calls.

We used to do that (except for the checks with whether inside macro calls or not), we moved it out because that function is used by lowering making it unnecessarily more expensive. So we should keep validation out of that.

because parse_or_expand() is defined in hir-expand, but to correctly identify whether something is inside macro we need hir-def.

Yep, that's the main issue, in our setup we need name resolution to figure out whether an attribute is actually expanding or not.

We should definitely consider caching validation errors though, as walking the "fully expanded tree" sounds fairly expensive for big files

[ChayimFriedman2]

Diagnostics in general are fairly expensive for big files, hir/src/lib.rs always felt a bit sluggish and I have a hunch this is because of diagnostics.