Remove `SymbolStr`
This was originally proposed in https://github.com/rust-lang/rust/pull/74554#discussion_r466203544. As well as removing the icky `SymbolStr` type, it allows the removal of a lot of `&` and `*` occurrences.
Best reviewed one commit at a time.
r? `@oli-obk`
`Resolver::next_node_id` converts a `u32` to a `usize` (which is
possibly bigger), does a checked add, and then converts the result back
to a `u32`. The `usize` conversion completely subverts the checked add!
This commit removes the conversion to/from `usize`.
Emit description of the ambiguity as a note.
Co-authored-by: Noah Lev <camelidcamel@gmail.com>
Co-authored-by: Vadim Petrochenkov <vadim.petrochenkov@gmail.com>
resolve: Use `NameBinding` for local variables and generic parameters
`NameBinding` is a structure used for representing any name introduction (an item, or import, or even a built-in).
Except that local variables and generic parameters weren't represented as `NameBinding`s, for this reason they requires separate paths in name resolution code in several places.
This PR introduces `NameBinding`s for local variables as well and simplifies all the code working with them leaving only the `NameBinding` paths.
Adopt let_else across the compiler
This performs a substitution of code following the pattern:
```
let <id> = if let <pat> = ... { identity } else { ... : ! };
```
To simplify it to:
```
let <pat> = ... { identity } else { ... : ! };
```
By adopting the `let_else` feature (cc #87335).
The PR also updates the syn crate because the currently used version of the crate doesn't support `let_else` syntax yet.
Note: Generally I'm the person who *removes* usages of unstable features from the compiler, not adds more usages of them, but in this instance I think it hopefully helps the feature get stabilized sooner and in a better state. I have written a [comment](https://github.com/rust-lang/rust/issues/87335#issuecomment-944846205) on the tracking issue about my experience and what I feel could be improved before stabilization of `let_else`.
Index and hash HIR as part of lowering
Part of https://github.com/rust-lang/rust/pull/88186
~Based on https://github.com/rust-lang/rust/pull/88880 (see merge commit).~
Once HIR is lowered, it is later indexed by the `index_hir` query and hashed for `crate_hash`. This PR moves those post-processing steps to lowering itself. As a side objective, the HIR crate data structure is refactored as an `IndexVec<LocalDefId, Option<OwnerInfo<'hir>>>` where `OwnerInfo` stores all the relevant information for an HIR owner.
r? `@michaelwoerister`
cc `@petrochenkov`
This performs a substitution of code following the pattern:
let <id> = if let <pat> = ... { identity } else { ... : ! };
To simplify it to:
let <pat> = ... { identity } else { ... : ! };
By adopting the let_else feature.
avoid suggesting the same name
sort candidates
fix a message
use `opt_def_id` instead of `def_id`
move `find_similarly_named_module_or_crate` to rustc_resolve/src/diagnostics.rs
It was previously cached for modules loaded from `fn get_module`, but not for modules loaded from `fn build_reduced_graph_for_external_crate_res`.
This also makes all foreign modules use their real parent, span and expansion instead of possibly a parent/span/expansion of their reexport.
An ICE happening on attempt to decode expansions for foreign enums and traits is avoided.
Also local enums and traits are now added to the module map.
Cleanup lower_generics_mut and make span be the bound itself
Closes#86298 (supersedes those changes)
r? `@cjgillot` since you reviewed the other PR
(Used wrong branch for #89338)
Do not suggest importing inaccessible items
Fixes#88472. For this example:
```rust
mod a {
struct Foo;
}
mod b {
type Bar = Foo;
}
```
rustc currently emits:
```
error[E0412]: cannot find type `Foo` in this scope
--> test.rs:6:16
|
6 | type Bar = Foo;
| ^^^ not found in this scope
|
help: consider importing this struct
|
6 | use a::Foo;
|
```
this is incorrect, as applying this suggestion leads to
```
error[E0603]: struct `Foo` is private
--> test.rs:6:12
|
6 | use a::Foo;
| ^^^ private struct
|
note: the struct `Foo` is defined here
--> test.rs:2:5
|
2 | struct Foo;
| ^^^^^^^^^^^
```
With my changes, I get:
```
error[E0412]: cannot find type `Foo` in this scope
--> test.rs:6:16
|
6 | type Bar = Foo;
| ^^^ not found in this scope
|
= note: this struct exists but is inaccessible:
a::Foo
```
As for the wildcard mentioned in #88472, I would argue that the warning is actually correct, since the import _is_ unused. I think the real issue is the wrong suggestion, which I have fixed here.
Suggest similarly named associated items in trait impls
Fix#85942
Previously, the compiler didn't suggest similarly named associated items unlike we do in many situations. This patch adds such diagnostics for associated functions, types, and constants.
Previously, the compiler didn't suggest similarly named associated items
unlike we do in many situations. This patch adds such diagnostics for
associated functions, types and constants.
The `Option<Module>` version is supported for the case where we don't know whether the `DefId` refers to a module or not.
Non-local traits and enums are also correctly found now.
Encode spans relative to the enclosing item
The aim of this PR is to avoid recomputing queries when code is moved without modification.
MCP at https://github.com/rust-lang/compiler-team/issues/443
This is achieved by :
1. storing the HIR owner LocalDefId information inside the span;
2. encoding and decoding spans relative to the enclosing item in the incremental on-disk cache;
3. marking a dependency to the `source_span(LocalDefId)` query when we translate a span from the short (`Span`) representation to its explicit (`SpanData`) representation.
Since all client code uses `Span`, step 3 ensures that all manipulations
of span byte positions actually create the dependency edge between
the caller and the `source_span(LocalDefId)`.
This query return the actual absolute span of the parent item.
As a consequence, any source code motion that changes the absolute byte position of a node will either:
- modify the distance to the parent's beginning, so change the relative span's hash;
- dirty `source_span`, and trigger the incremental recomputation of all code that
depends on the span's absolute byte position.
With this scheme, I believe the dependency tracking to be accurate.
For the moment, the spans are marked during lowering.
I'd rather do this during def-collection,
but the AST MutVisitor is not practical enough just yet.
The only difference is that we attach macro-expanded spans
to their expansion point instead of the macro itself.
Remove `Session.used_attrs` and move logic to `CheckAttrVisitor`
Instead of updating global state to mark attributes as used,
we now explicitly emit a warning when an attribute is used in
an unsupported position. As a side effect, we are to emit more
detailed warning messages (instead of just a generic "unused" message).
`Session.check_name` is removed, since its only purpose was to mark
the attribute as used. All of the callers are modified to use
`Attribute.has_name`
Additionally, `AttributeType::AssumedUsed` is removed - an 'assumed
used' attribute is implemented by simply not performing any checks
in `CheckAttrVisitor` for a particular attribute.
We no longer emit unused attribute warnings for the `#[rustc_dummy]`
attribute - it's an internal attribute used for tests, so it doesn't
mark sense to treat it as 'unused'.
With this commit, a large source of global untracked state is removed.
Instead of updating global state to mark attributes as used,
we now explicitly emit a warning when an attribute is used in
an unsupported position. As a side effect, we are to emit more
detailed warning messages (instead of just a generic "unused" message).
`Session.check_name` is removed, since its only purpose was to mark
the attribute as used. All of the callers are modified to use
`Attribute.has_name`
Additionally, `AttributeType::AssumedUsed` is removed - an 'assumed
used' attribute is implemented by simply not performing any checks
in `CheckAttrVisitor` for a particular attribute.
We no longer emit unused attribute warnings for the `#[rustc_dummy]`
attribute - it's an internal attribute used for tests, so it doesn't
mark sense to treat it as 'unused'.
With this commit, a large source of global untracked state is removed.
Fix use placement for suggestions near main.
This fixes an edge case for the suggestion to add a `use`. When running with `--test`, the `main` function will be annotated with an `#[allow(dead_code)]` attribute. The `UsePlacementFinder` would end up using the dummy span of that synthetic attribute. If there are top-level inner attributes, this would place the `use` in the wrong position. The solution here is to ignore attributes with dummy spans.
In the process of working on this, I discovered that the `use_suggestion_placement` test was broken. `UsePlacementFinder` is unaware of active attributes. Attributes like `#[derive]` don't exist in the AST since they are removed. Fixing that is difficult, since the AST does not retain enough information. I considered trying to place the `use` towards the top of the module after any `extern crate` items, but I couldn't find a way to get a span for the start of a module block (the `mod` span starts at the `mod` keyword, and it seems tricky to find the spot just after the opening bracket and past inner attributes). For now, I just put some comments about the issue. This appears to have been a known issue in #44215 where the test for it was introduced, and the fix seemed to be deferred to later.
Disallow shadowing const parameters
This pull request fixes#85348. Trying to shadow a `const` parameter as follows:
```rust
fn foo<const N: i32>() {
let N @ _ = 0;
}
```
currently causes an ICE. With my changes, I get:
```
error[E0530]: let bindings cannot shadow const parameters
--> test.rs:2:9
|
1 | fn foo<const N: i32>() {
| - the const parameter `N` is defined here
2 | let N @ _ = 0;
| ^ cannot be named the same as a const parameter
error: aborting due to previous error
```
This is the same error you get when trying to shadow a constant:
```rust
const N: i32 = 0;
let N @ _ = 0;
```
```
error[E0530]: let bindings cannot shadow constants
--> src/lib.rs:3:5
|
2 | const N: i32 = 0;
| ----------------- the constant `N` is defined here
3 | let N @ _ = 0;
| ^ cannot be named the same as a constant
error: aborting due to previous error
```
The reason for disallowing shadowing in both cases is described [here](https://github.com/rust-lang/rust/issues/33118#issuecomment-233962221) (the comment there only talks about constants, but the same reasoning applies to `const` parameters).
This PR implements span quoting, allowing proc-macros to produce spans
pointing *into their own crate*. This is used by the unstable
`proc_macro::quote!` macro, allowing us to get error messages like this:
```
error[E0412]: cannot find type `MissingType` in this scope
--> $DIR/auxiliary/span-from-proc-macro.rs:37:20
|
LL | pub fn error_from_attribute(_args: TokenStream, _input: TokenStream) -> TokenStream {
| ----------------------------------------------------------------------------------- in this expansion of procedural macro `#[error_from_attribute]`
...
LL | field: MissingType
| ^^^^^^^^^^^ not found in this scope
|
::: $DIR/span-from-proc-macro.rs:8:1
|
LL | #[error_from_attribute]
| ----------------------- in this macro invocation
```
Here, `MissingType` occurs inside the implementation of the proc-macro
`#[error_from_attribute]`. Previosuly, this would always result in a
span pointing at `#[error_from_attribute]`
This will make many proc-macro-related error message much more useful -
when a proc-macro generates code containing an error, users will get an
error message pointing directly at that code (within the macro
definition), instead of always getting a span pointing at the macro
invocation site.
This is implemented as follows:
* When a proc-macro crate is being *compiled*, it causes the `quote!`
macro to get run. This saves all of the sapns in the input to `quote!`
into the metadata of *the proc-macro-crate* (which we are currently
compiling). The `quote!` macro then expands to a call to
`proc_macro::Span::recover_proc_macro_span(id)`, where `id` is an
opaque identifier for the span in the crate metadata.
* When the same proc-macro crate is *run* (e.g. it is loaded from disk
and invoked by some consumer crate), the call to
`proc_macro::Span::recover_proc_macro_span` causes us to load the span
from the proc-macro crate's metadata. The proc-macro then produces a
`TokenStream` containing a `Span` pointing into the proc-macro crate
itself.
The recursive nature of 'quote!' can be difficult to understand at
first. The file `src/test/ui/proc-macro/quote-debug.stdout` shows
the output of the `quote!` macro, which should make this eaier to
understand.
This PR also supports custom quoting spans in custom quote macros (e.g.
the `quote` crate). All span quoting goes through the
`proc_macro::quote_span` method, which can be called by a custom quote
macro to perform span quoting. An example of this usage is provided in
`src/test/ui/proc-macro/auxiliary/custom-quote.rs`
Custom quoting currently has a few limitations:
In order to quote a span, we need to generate a call to
`proc_macro::Span::recover_proc_macro_span`. However, proc-macros
support renaming the `proc_macro` crate, so we can't simply hardcode
this path. Previously, the `quote_span` method used the path
`crate::Span` - however, this only works when it is called by the
builtin `quote!` macro in the same crate. To support being called from
arbitrary crates, we need access to the name of the `proc_macro` crate
to generate a path. This PR adds an additional argument to `quote_span`
to specify the name of the `proc_macro` crate. Howver, this feels kind
of hacky, and we may want to change this before stabilizing anything
quote-related.
Additionally, using `quote_span` currently requires enabling the
`proc_macro_internals` feature. The builtin `quote!` macro
has an `#[allow_internal_unstable]` attribute, but this won't work for
custom quote implementations. This will likely require some additional
tricks to apply `allow_internal_unstable` to the span of
`proc_macro::Span::recover_proc_macro_span`.
Issue 81508 fix
Fix#81508
**Problem**: When variable name is used incorrectly as path, error and warning point to undeclared/unused name, when in fact the name is used, just incorrectly (should be used as a variable, not part of a path).
**Summary for fix**: When path resolution errs, diagnostics checks for variables in ```ValueNS``` that have the same name (e.g., variable rather than path named Foo), and adds additional suggestion that user may actually intend to use the variable name rather than a path.
The fix does not suppress or otherwise change the *warning* that results. I did not find a straightforward way in the code to modify this, but would love to make changes here as well with any guidance.
Previously, the types looked like this:
- None means this is not an associated item (but may be a variant field)
- Some(Err) means this is known to be an error. I think the only way that can happen is if it resolved and but you had your own anchor.
- Some(Ok(_, None)) was impossible.
Now, this returns a nested Option and does the error handling and
fiddling with the side channel in the caller. As a side-effect, it also
removes duplicate error handling.
This has one small change in behavior, which is that
`resolve_primitive_associated_item` now goes through `variant_field` if
it fails to resolve something. This is not ideal, but since it will be
quickly rejected anyway, I think the performance hit is worth the
cleanup.
This also fixes a bug where struct fields would forget to set the side
channel, adds a test for the bug, and ignores `private_intra_doc_links`
in rustc_resolve (since it's always documented with
--document-private-items).
resolve: Partially unify early and late scope-relative identifier resolution
Reuse `early_resolve_ident_in_lexical_scope` instead of a chunk of code in `resolve_ident_in_lexical_scope` doing the same job.
`early_resolve_ident_in_lexical_scope`/`visit_scopes` had to be slightly extended to be able to 1) start from a specific module instead of the current parent scope and 2) report one deprecation lint.
`early_resolve_ident_in_lexical_scope` still doesn't support walking through "ribs", that part is left in `resolve_ident_in_lexical_scope` (moreover, I'm pretty sure it's buggy, but that's a separate issue, cc https://github.com/rust-lang/rust/issues/52389 at least).
Crate root is sufficiently different from `mod` items, at least at syntactic level.
Also remove customization point for "`mod` item or crate root" from AST visitors.
Special treatment like this was necessary before `pub(restricted)` had been implemented and only two visibilities existed - `pub` and non-`pub`.
Now it's no longer necessary and the desired behavior follows from `pub(restricted)`-style visibilities naturally assigned to enum variants and trait items.
Refactor `PrimitiveTypeTable` for Clippy
I removed `PrimitiveTypeTable` and added `PrimTy::ALL` and `PrimTy::from_name` in its place. This allows Clippy to use `PrimTy::from_name` for the `builtin_type_shadow` lint, and a `const` list of primitive types is deleted from Clippy code (the goal). All changes should be a little faster, if anything.
resolve: Simplify collection of traits in scope
"Traits in scope" for a given location are collected by walking all scopes in type namespace, collecting traits in them and pruning traits that don't have an associated item with the given name and namespace.
Previously we tried to prune traits using some kind of hygienic resolution for associated items, but that was complex and likely incorrect, e.g. in #80762 correction to visibilites of trait items caused some traits to not be in scope anymore.
I previously had some comments and concerns about this in https://github.com/rust-lang/rust/pull/65351.
In this PR we are doing some much simpler pruning based on `Symbol` and `Namespace` comparisons, it should be enough to throw away 99.9% of unnecessary traits.
It is not necessary for pruning to be precise because for trait aliases, for example, we don't do any pruning at all, and precise hygienic resolution for associated items needs to be done in typeck anyway.
The somewhat unexpected effect is that trait imports introduced by macros 2.0 now bring traits into scope due to the removed hygienic check on associated item names.
I'm not sure whether it is desirable or not, but I think it's acceptable for now.
The old check was certainly incorrect because macros 2.0 did bring trait aliases into scope.
If doing this is not desirable, then we should come up with some other way to avoid bringing traits from macros 2.0 into scope, that would accommodate for trait aliases as well.
---
The PR also contains a couple of pure refactorings
- Scope walk is done by using `visit_scopes` instead of a hand-rolled version.
- Code is restructured to accomodate for rustdoc that also wants to query traits in scope, but doesn't want to filter them by associated items at all.
r? ```@matthewjasper```
resolve: Simplify built-in macro table
We don't use full `SyntaxExtension`s from the table, only `SyntaxExtensionKind`s, and `Ident` in `register_builtin_macro` always had dummy span. This PR removes unnecessary data from the table and related function signatures.
Noticed when reviewing #80850.
resolve: Scope visiting doesn't need an `Ident`
Resolution scope visitor (`fn visit_scopes`) currently takes an `Ident` parameter, but it doesn't need a full identifier, or even its span, it only needs the `SyntaxContext` part.
The `SyntaxContext` part is necessary because scope visitor has to jump to macro definition sites, so it has to be directed by macro expansion information somehow.
I think it's clearer to pass only the necessary part.
Yes, usually visiting happens as a part of an identifier resolution, but in cases like collecting traits in scope (#80765) or collecting typo suggestions that's not the case.
r? `@matthewjasper`
Allow #[rustc_builtin_macro = "name"]
This adds the option of specifying the name of a builtin macro in the `#[rustc_builtin_macro]` attribute: `#[rustc_builtin_macro = "name"]`.
This makes it possible to have both `std::panic!` and `core::panic!` as a builtin macro, by using different builtin macro names for each. This is needed to implement the edition-specific behaviour of the panic macros of RFC 3007.
Also removes `SyntaxExtension::is_derive_copy`, as the macro name (e.g. `sym::Copy`) is now tracked and provides that information directly.
r? ``@petrochenkov``
This makes it possible to have both std::panic and core::panic as a
builtin macro, by using different builtin macro names for each.
Also removes SyntaxExtension::is_derive_copy, as the macro name (e.g.
sym::Copy) is now tracked and provides that information directly.
On structured suggestion for `let` -> `const` and `const` -> `let`, use
a proper `Span` and update tests to check the correct application.
Follow up to #80012.
* Rename `ModuleData.normal_ancestor_id` to `nearest_parent_mod`
`normal_ancestor_id` is a very confusing name if you don't already
understand what it means. Adding docs helps, but using a clearer and
more obvious name is also important.
* Rename `Resolver::nearest_mod_parent` to `nearest_parent_mod`
* Add more docs
remove unused return type of dropck::check_drop_obligations()
don't wrap return type in Option in get_macro_by_def_id() since we would always return Some(..)
remove redundant return type of back::write::optimize()
don't Option-wrap return type of compute_type_parameters() since we always return Some(..)
don't return empty Result in assemble_generator_candidates()
don't return empty Result in assemble_closure_candidates()
don't return empty result in assemble_fn_pointer_candidates()
don't return empty result in assemble_candidates_from_impls()
don't return empty result in assemble_candidates_from_auto_impls()
don't return emtpy result in assemble_candidates_for_trait_alias()
don't return empty result in assemble_builtin_bound_candidates()
don't return empty results in assemble_extension_candidates_for_traits_in_scope() and assemble_extension_candidates_for_trait()
remove redundant wrapping of return type of StripItem::strip() since it always returns Some(..)
remove unused return type of assemble_extension_candidates_for_all_traits()
Previously, this code treated enum fields' visibility as if they were
struct fields. However, that's not correct because the visibility of a
struct field with `ast::VisibilityKind::Inherited` is private to the
module it's defined in, whereas the visibility of an *enum* field with
`ast::VisibilityKind::Inherited` is the visibility of the enum it
belongs to.
Use `DroplessArena` where we know the type doesn't need drop
This PR uses a single `DroplessArena` in resolve instead of three separate `TypedArena`s.
`DroplessArena` checks that the type indeed doesn't need drop, so in case the types change, this will result in visible failures.
* Change error message for type param in a const expression when using
min_const_generics
* Change ParamInNonTrivialAnonConst to contain an extra bool used for
distinguishing whether the passed-in symbol is a type or a value.
Give better diagnostic when using a private tuple struct constructor
Fixes#75907
Some notes:
1. This required some deep changes, including removing a Copy impl for PatKind. If some tests fail, I would still appreciate review on the overall approach
2. this only works with basic patterns (no wildcards for example), and fails if there is any problems getting the visibility of the fields (i am not sure what the failure that can happen in resolve_visibility_speculative, but we check the length of our fields in both cases against each other, so if anything goes wrong, we fall back to the worse error. This could be extended to more patterns
3. this does not yet deal with #75906, but I believe it will be similar
4. let me know if you want more tests
5. doesn't yet at the suggestion that `@yoshuawuyts` suggested at the end of their issue, but that could be added relatively easily (i believe)
Improve unresolved use error message
"use of undeclared type or module `foo`" doesn't mention that it could be a crate.
This error can happen when users forget to add a dependency to `Cargo.toml`, so I think it's important to mention that it could be a missing crate.
I've used a heuristic based on Rust's naming conventions. It complains about an unknown type if the ident starts with an upper-case letter, and crate or module otherwise. It seems to work very well. The expanded error help covers both an unknown type and a missing crate case.
Previously, this would say no such macro existed, but this was
misleading, since the macro _did_ exist, it was just already seen.
- Say where the macro was previously defined
- Add long-form error message
