proc_macro/bridge: stop using a remote object handle for proc_macro Ident and Literal
This is the fourth part of https://github.com/rust-lang/rust/pull/86822, split off as requested in https://github.com/rust-lang/rust/pull/86822#pullrequestreview-1008655452. This patch transforms the `Ident` and `Group` types into structs serialized over IPC rather than handles.
Symbol values are interned on both the client and server when deserializing, to avoid unnecessary string copies and keep the size of `TokenTree` down. To do the interning efficiently on the client, the proc-macro crate is given a vendored version of the fxhash hasher, as `SipHash` appeared to cause performance issues. This was done rather than depending on `rustc_hash` as it is unfortunately difficult to depend on crates from within `proc_macro` due to it being built at the same time as `std`.
In addition, a custom arena allocator and symbol store was also added, inspired by those in `rustc_arena` and `rustc_span`. To prevent symbol re-use across multiple invocations of a macro on the same thread, a new range of `Symbol` names are used for each invocation of the macro, and symbols from previous invocations are cleaned-up.
In order to keep `Ident` creation efficient, a special ASCII-only case was added to perform ident validation without using RPC for simple identifiers. Full identifier validation couldn't be easily added, as it would require depending on the `rustc_lexer` and `unicode-normalization` crates from within `proc_macro`. Unicode identifiers are validated and normalized using RPC.
See the individual commit messages for more details on trade-offs and design decisions behind these patches.
Currently `#![forbid(unused_qualifications)]` is incompatible with all
derive's because we add `#[allow(unused_qualifications)]` in all
generated impl's.
- Use `expr.hir_id.owner` instead of `self.tcx.parent_module(expr.hir_id)`
- Use `.type_at()` instead of `.first()` + `.expect_ty()`
- Use single `.find()` with `&&` condition
Co-authored-by: Michael Goulet <michael@errs.io>
Rollup of 7 pull requests
Successful merges:
- #98839 (Add assertion that `transmute_copy`'s U is not larger than T)
- #98998 (Remove branch target prologues from `#[naked] fn`)
- #99198 (add missing null ptr check in alloc example)
- #99344 (rustdoc: avoid inlining items with duplicate `(type, name)`)
- #99351 (Use `typeck_results` to get accurate qpath res for arg mismatch error)
- #99378 (interpret/visitor: add missing early return)
- #99394 (Add regression test for #95230)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
This method is still only used for Literal::subspan, however the
implementation only depends on the Span component, so it is simpler and
more efficient for now to pass down only the information that is needed.
In the future, if more information about the Literal is required in the
implementation (e.g. to validate that spans line up as expected with
source text), that extra information can be added back with extra
arguments.
This builds on the symbol infrastructure built for `Ident` to replicate
the `LitKind` and `Lit` structures in rustc within the `proc_macro`
client, allowing literals to be fully created and interacted with from
the client thread. Only parsing and subspan operations still require
sync RPC.
Doing this for all unicode identifiers would require a dependency on
`unicode-normalization` and `rustc_lexer`, which is currently not
possible for `proc_macro` due to it being built concurrently with `std`
and `core`. Instead, ASCII identifiers are validated locally, and an RPC
message is used to validate unicode identifiers when needed.
String values are interned on the both the server and client when
deserializing, to avoid unnecessary copies and keep Ident cheap to copy and
move. This appears to be important for performance.
The client-side interner is based roughly on the one from rustc_span, and uses
an arena inspired by rustc_arena.
RPC messages passing symbols always include the full value. This could
potentially be optimized in the future if it is revealed to be a
performance bottleneck.
Despite now having a relevant implementaion of Display for Ident, ToString is
still specialized, as it is a hot-path for this object.
The symbol infrastructure will also be used for literals in the next
part.
Unfortunately, as it is difficult to depend on crates from within proc_macro,
this is done by vendoring a copy of the hasher as a module rather than
depending on the rustc_hash crate.
This probably doesn't have a substantial impact up-front, however will be more
relevant once symbols are interned within the proc_macro client.
Use `typeck_results` to get accurate qpath res for arg mismatch error
Improves error message from "function" to actually what we're calling (e.g. enum variant constrcutor) in a few cases 😸
add missing null ptr check in alloc example
`alloc` can return null on OOM, if I understood correctly. So we should never just deref a pointer we get from `alloc`.
Remove branch target prologues from `#[naked] fn`
This patch hacks around rust-lang/rust#98768 for now via injecting appropriate attributes into the LLVMIR we emit for naked functions. I intend to pursue this upstream so that these attributes can be removed in general, but it's slow going wading through C++ for me.
Add assertion that `transmute_copy`'s U is not larger than T
This is called out as a safety requirement in the docs, but because knowing this can be done at compile time and constant folded (just like the `align_of` branch is removed), we can just panic here.
I've looked at the asm (using `cargo-asm`) of a function that both is correct and incorrect, and the panic is completely removed, or is unconditional, without needing build-std.
I don't expect this to cause much breakage in the wild. I scanned through https://miri.saethlin.dev/ub for issues that would look like this (error: Undefined Behavior: memory access failed: alloc1768 has size 1, so pointer to 8 bytes starting at offset 0 is out-of-bounds), but couldn't find any.
That doesn't rule out it happening in crates tested that fail earlier for some other reason, though, but it indicates that doing this is rare, if it happens at all. A crater run for this would need to be build and test, since this is a runtime thing.
Also added a few more transmute_copy tests.
`replace_bound_vars` fast path: check predicates, don't check consts
split out from #98900
`ty::Const` doesn't have precomputed type flags, so
computing `has_vars_bound_at_or_above` for constants
requires us to visit the const and its contained types
and constants. A noop fold should be pretty much equally as
fast so removing it prevents us from walking the constant twice
in case it contains bound vars.
r? `@jackh726`
Rearrange slice::split_mut to remove bounds check
Closes https://github.com/rust-lang/rust/issues/86313
Turns out that all we need to do here is reorder the bounds checks to convince LLVM that all the bounds checks can be removed. It seems like LLVM just fails to propagate the original length information past the first bounds check and into the second one. With this implementation it doesn't need to, each check can be proven inbounds based on the one immediately previous.
I've gradually convinced myself that this implementation is unambiguously better based on the above logic, but maybe this is still deserving of a codegen test?
Also the mentioned borrowck limitation no longer seems to exist.
`arena > Rc` for query results
The `Rc`s have to live for the whole duration as their count cannot go below 1 while stored as part of the query results.
By storing them in an arena we should save a bit of memory because we don't have as many independent allocations and also don't have to clone the `Rc` anymore.
Don't pass InferCtxt to WfPredicates
Simple cleanup. Infer vars will get passed up as obligations and shallowed resolved later. This actually improves one test output.
Rollup of 6 pull requests
Successful merges:
- #98383 (Remove restrictions on compare-exchange memory ordering.)
- #99350 (Be more precise when suggesting removal of parens on unit ctor)
- #99356 (Do not constraint TAITs when checking impl/trait item compatibility)
- #99360 (Do not ICE when we have `-Zunpretty=expanded` with invalid ABI)
- #99373 (Fix source code sidebar tree auto-expand)
- #99374 (Fix doc for `rchunks_exact`)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Fix source code sidebar tree auto-expand
Here is the bug:
It was happening because as soon as we found the file (from the URL), every item following it was then opened, even if it wasn't supposed to.
The GUI test ensures that it doesn't happen by adding two nested levels and ensuring only this path is "open".
r? ``@notriddle``
