Resolve: more preperation work for parallelizing the import resolution loop
This is basically rust-lang/rust#158845 but we do not:
- actually use `par_slice` because:
- we do not migrate `CmRefCell` to use `RwLocks` (yet) because of perf reasons.
The resolution loop is now in place instead of recollecting indeterminate imports.
r? @petrochenkov
Improve cross-namespace name diagnostics
Closerust-lang/rust#86290
Cross-namespace name matches now use the ordinary missing-name diagnostic while preserving its error code and suggestions. The diagnostic adds a note identifying the item found in another namespace, while same-namespace wrong-kind diagnostics remain unchanged.
The UI expectations cover cross-namespace combinations across the resolver suite, including preservation of existing suggestions.
Resolver: Record at least 1 ambiguous trait if main decl is not a trait.
Addresses rust-lang/rust#159476.
When recording traits of a particular module, it is possible that a trait hidden in the ambiguity declaration of a main declaration (struct, enum, ...), thus causing us to never record that trait. This caused us to report an error that methods of those traits are not in scope and that we do not report the `AMBIGUOUSLY_GLOB_IMPORTED_TRAIT` lint, which was the case in rust-lang/rust#159476.
We now record at least 1 trait in the collection of ambiguous declarations, such that we don't report that error and can report the relevant lint.
Also updated the linked issue of the lint in question.
r? @petrochenkov
Currently it is all still single threaded, but almost everything needed to make the loop use multithreaded code is done in this commit.
For performance reasons we do the resolution now in place instead of recollecting the indeterminate imports ([see benchmark pr](https://github.com/rust-lang/rust/pull/159387) and pr of this commit).
Populating external resolution tables now use `Once` to ensure only 1 thread builds the table and all others wait if they need it, this has no impact on single threaded behaviour.
`Cache(Ref)Cells` are not tackled yet because they are not needed here (and require extra work).
Overhaul `CfgTrace`/`CfgAttrTrace` handling
I noticed that `AttrItemKind` was a bit clunky and by the time I was finished I had a PR that:
- removes `AttrItemKind`;
- removes `sym::cfg_trace` and `sym::cfg_attr_trace`;
- overhauls how these attributes are handled;
- changes the terminology used for these attributes;
- fixes some erroneous comments;
- and a few other small improvements.
It's a sizeable change but I think it makes things a lot clearer. Details in individual commits.
r? @petrochenkov
AST attributes use "early parsed"/"parsed" terminology to refer to the
`CfgTrace` and `CfgAttrTrace` attributes. I think this terminology is
meant to echo the terminology used for HIR attributes, i.e.
`hir::Attribute::{Parsed,Unparsed}`, probably because
`hir::Attribute::Parsed` is used for attributes that aren't stored in a
token-based form.
But this naming is misleading. "Early parsed" attributes aren't parsed
at all because they are inserted by the compiler and cannot be written
in source code. There are also two comments that claim that these
attributes are kept in parsed form "so they don't have to be reparsed
every time they're used, for performance", which is simply incorrect.
This commit renames these as "synthetic" attributes, which better
reflects their nature. The commit also fixes the incorrect comments.
Note that `is_parsed_attribute` is unchanged, because it refers to the
HIR attribute meaning. (And the removal of the synthetic attributes from
it in the previous commit is now more obviously correct.)
This commit pushes further than the previous one, relying more heavily
on looking directly at `AttrKind::Parsed` to handle various cases.
Note that the removed clippy code was actually dead.
`AttrItem` currently contains an `AttrItemKind` which is either `Parsed`
or `Unparsed`. In the `Parsed` case the surrounding `AttrItem` is
basically fake, with a meaningless `unsafety` field, a synthetic
(non-ident) symbol, and an empty `tokens`.
This commit moves `Parsed` up one level to become a new variant of
`AttrKind`. It no longer needs to pretend to be an `AttrKind::Normal`,
and makes a number of things simpler:
- No `unparsed_ref` calls needed.
- Replaces the `attr_into_trace` and `Attribute::replace_args` mess with
the much simpler `convert_normal_to_parsed`.
The commit converts a numberof non-exhaustive `AttrKind` matches (those
that don't involve matching a single attribute) to exhaustive, for
future-proofing. It uses `unreachable!` on all the `AttrKind::Parsed`
cases that aren't hit by the full test suite.
Resolver: extract `use_injections` out of main `Resolver`
Prep work for rust-lang/rust#158845. Extract the `use_injections` field out of the resolver and into the `LateResolutionVisitor`. It can then be passed into the `report_errors` pass, so no behaviour is changed.
This allows the `Resolver` to be `DynSync`, see https://github.com/rust-lang/rust/pull/158845#discussion_r3579712892.
r? @petrochenkov
Rename ModDefId to ModId and use more
Addresses some FIXMEs.
The `ModDefId` was used pretty sparingly, and honestly I wondered if it should exist at all. After making these changes, I think the type is worth it. Most notably it is now used in `Visibility::Restricted`.
I first renamed `ModDefId` to `ModId` since I think the former is over-specific, and also this is more in line with `BodyId` or `OwnerId`.
I'll wait for initial feedback before fixing rustdoc/clippy.
resolve: Inherit eager invocation parents
Fixesrust-lang/rust#159233
`format!` eagerly expands its first arg. In this case that path ran into a glob delegation from `fn_delegation`, and resolver tried to read `invocation_parents[invoc_id]` for an eager invocation that never went through reduced-graph collection. So a bad input got an ICE instead of normal errors.
This makes eager invocations copy `InvocationParent` from the eager expansion root, matching the parent-scope fallback already there. imo this is the right place to fix it: idk of a cleaner split where the scope and parent def do not drift apart. fyi the UI regression is the reported case, btw, and it still emits the expected user-facing errors without the panic.
Shrink `ast::Expr64`
From 72 bytes to 64 bytes, by boxing `ForLoop`, which is the biggest
variant.
There are now 11 `ExprKind` variants that are 31 bytes, so future
improvements here will be very difficult.
Store `DefId` instead of `EiiDecl` in `EiiImplResolution::Known`
Fixes the `FIXME(eii)` about removing the extern target from the encoded decl.
`Known(EiiDecl)` serialized the same `EiiDecl` N+1 times per entry (once as the declaration, once per Known impl). Changing to `Known(DefId)` eliminates that redundancy — the `EiiDecl` is recoverable via `tcx.externally_implementable_items()` or a local lookup map, consistent with how Rust metadata uses `DefId` as a reference elsewhere.
I believe there are benefits in both performance and size(full `EiiDecl` to single `DefId`, and O(N) to O(1) lookup).
Also fixes a soundness gap(I think?) in `check_attr.rs` where the `Known` branch skipped `impl_unsafe` checking, and replaces an `.expect()` in `codegen_attrs.rs` with `a let Some` pattern.
tracking issues: rust-lang/rust#125418
r? @jdonszelmann
resolve: fix effective visibilities for items in ambiguous glob sets
Fixesrust-lang/rust#159038 (1.96.1 → 1.97.0 regression; details there).
When an item is glob-imported twice at different visibilities, effective visibility was computed from whichever declaration arrived first, not the most visible one. An exported item could then get no MIR encoded, and downstream crates fail with ``missing optimized MIR``.
Fix: also walk the most visible declaration's re-export chain (`update_decl_chain`, recursing into `ambiguity_vis_max`). Lint behavior unchanged. Two regression tests added; `tests/ui/{imports,privacy,resolve}` pass with every rust-lang/rust#154149 / rust-lang/rust#156284 test unmodified.
@rustbot label +A-resolve +A-visibility +T-compiler +regression-from-stable-to-stable
From 72 bytes to 64 bytes, by boxing `ForLoop`, which is the biggest
variant.
There are now 11 `ExprKind` variants that are 31 bytes, so future
improvements here will be very difficult.
diagnostics: suggest generic_const_args for const ops
fixesrust-lang/rust#156729
when const ops hit generic params, like `[u8; size_of::<self>()]`, the diagnostic only points at `generic_const_exprs`. imo that's a stale nudge now that `generic_const_args` and `type const` items are the path we want people trying.
add help for `generic_const_args` and `type const` items in the resolve and hir_ty_lowering paths. also skip the old gce suggestion once `min_generic_const_args` is enabled, and emit both suggestions for the `self` alias path so the "alternatively" wording doesn't hang there by itself. includes the regression test from the issue. lgtm.
Resolver: Wrap arenas in `WorkerLocal`
In preperation of parallel import resolution rust-lang/rust#158845 this pr wraps the resolver arenas in `WorkerLocal` to ensure we don't need any synchronization to access an arena. This is common in the compiler.
Also, when building the resolution table of an external module, we first create a seperate table and fill that one, after which we replace the empty table with that new table. (Prep Work for rust-lang/rust#158845)
And a manual inline of `define_extern`.
r? @petrochenkov
merge DefKind::InlineConst into AnonConst
This is a closely related followup to https://github.com/rust-lang/rust/pull/158375 (a condition of merging that PR was doing this as a followup)
This merge conflicts with https://github.com/rust-lang/rust/pull/158617 ; prefer merging that one first please~
This PR is within the general goal of const generics / `generic_const_args` and perhaps specifically https://github.com/rust-lang/project-const-generics/issues/108 but is mostly a code cleanup rather than implementing any particular feature
---
Anon consts (the `1 + 2` in `let x: [u8; 1 + 2];`) are closely related to inline consts (the `const {}` in `let x = const { 1 + 2 };`). They are especially related now that both can be used in the type system (by the above PR) and should be treated identically in that scenario. In general, they should be treated the same, as evidenced by the number of matches touched in this PR that just match on both. This PR merges the two DefKinds into one, mostly arbitrarily and bikesheddily choosing to use the name `AnonConst` for the merged concept (e.g. both `DefKind::InlineConst` and `DefKind::AnonConst` used `ast::AnonConst`, so `ast::AnonConst` should probably be named whatever the merged `DefKind` concept is named).
When you absolute must distinguish between the two, `tcx.anon_const_kind` allows you to do so - however, anon consts/inline consts in the type system intentionally appear identically under `anon_const_kind`. Indeed, the places where we distinguish between the two raised my eyebrows a few times when doing this PR, and being a little more explicit and loud about it is helpful for code clarity, IMO.
r? @BoxyUwU
Point users at `#![feature(generic_const_args)]` and `type const` items
when they hit the "generic parameters may not be used in const operations"
error, not just `generic_const_exprs`.
Specifically, from `Block`, `Pat`, `Path`, `Ty`, `Visibility`.
This field is usually `None`. It only gets filled in for paths that
might later be re-tokenized via `TokenStream::from_ast`, e.g. because of
macro or cfg use. All such cases go through a single place:
`parse_nonterminal`.
This commit changes `parse_nonterminal` to store the non-`None` lazy
attr token stream *next* to the node in `ParseNtResult` (using the new
`WithTokens` type) instead of *inside* the parsed node.
Along with some minor changes to the relevant `HasTokens` and `HasAttrs`
impls, this is enough for things to work. These AST nodes all shrink by
8 bytes, as do various other nodes that contain nodes of these types.
A guide to the changes:
- `compiler/rustc_ast/src/tokenstream.rs` has the new `WithTokens` type.
- `compiler/rustc_parse/src/parser/nonterminal.rs`,
`compiler/rustc_expand/src/mbe/transcribe.rs` and
`compiler/rustc_parse/src/parser/mod.rs` have minor changes due to the
changes to `ParseNtResult`.
- `compiler/rustc_ast/src/ast_traits.rs` has the `HasTokens`/`HasAttrs`
changes.
- Everything else is just mechanical changes for the removal of the
`tokens` field.
allow mGCA const arguments to fall back to anon consts
makes good progress on https://github.com/rust-lang/project-const-generics/issues/108
`min_generic_const_args` (mGCA) is all about "is this const lowered to a const item with a body, or, is it "directly" represented and visible in the type system?"
Here's various answers to the question of "can we represent this const arg directly?" - if we answer "no", then we fall back to an anon const (or error, sometimes, depending on context)
on **stable**, the full list of things can be directly represented is (... it's just the one thing)
- paths to generic const parameters
on `main`, under mGCA, **before** this PR:
- absolutely everything, if something cannot be represented directly, compiler error
- `const { }` gives you an escape hatch to go back to being an anon const
on macro**ful** gca, the directly represented things will be:
- paths to const parameters
- `direct_const_arg!` macro
on macro**less** gca, the directly represented things will be:
- paths to const parameters
- `direct_const_arg!` macro (not *particularly* useful under macro**less** gca)
- struct expressions, arrays, blah blah
- currently, paths to *anything*, even if that produces a compiler error later down the line (potential followup work here...)
this PR implements macro**less** gca, with the intent of a quick follow-up introducing a "macroless gca" feature and transitioning `#[feature(min_generic_const_args)]` to be macro**ful** gca
Also of note is that this PR removes the logic to skip generating "fake" DefIds for directly represented AnonConsts under mGCA, we now always generate a DefId when encountering an AnonConst. This lines up with stable, which has a fake AnonConst DefId for a directly represented plain path generic parameter (i.e. the bullet point under the "stable" entry above).
r? @BoxyUwU
rustdoc: do not include extra stuff in span
This change prevents our lints from returning a span with stuff in it that isn't actually part of the doc comment. When that happens, it returns `None` instead.
Fixes https://github.com/rust-lang/rust-clippy/issues/16169, since the bug was reported against the `clippy::doc_paragraphs_missing_punctuation` lint but is actually a bug in `source_span_for_markdown_range_inner`.
Since PR 154149, when one item is glob-imported into a module twice with
different visibilities, the first-arrived declaration stays in the
resolution slot and the most visible declaration of the ambiguous glob
set is only recorded in `ambiguity_vis_max`. `DeclData::vis()` returns
the max, so name resolution, metadata reexports and
`cross_crate_inlinable` export the item at the maximum visibility, but
`set_bindings_effective_visibilities` walked only the slot-resident
declaration's reexport chain. When the restricted route arrives first,
the definition's effective visibility caps at the restricted
visibility while the item is still exported: spurious dead_code, the
item missing from reachable_set, should_encode_mir returning false, and
downstream crates failing with "missing optimized MIR" (a 1.96.1 ->
1.97.0 stable-to-stable regression).
Generalize the one-level `ambiguity_vis_max` update that PR 154149 added
in `update_import` (to keep the most visible import from being reported
as unused) into a walk of that declaration's whole reexport chain:
extract the chain walk into `update_decl_chain` and recurse into
`ambiguity_vis_max` at every hop, so the most visible declaration
drives the effective visibility of everything on its route, including
the final definition. Updates are monotone, so the dual walk is
order-independent. The `ambiguous_import_visibilities` lint and the
PR 156284 diagnostic suppression are untouched.
panstromek