chore(config): remove `invocationLocation` in favor of `invocationStrategy`
These flags were added to help rust-analyzer integrate with repos requiring non-Cargo invocations. The consensus is that having two independent settings are no longer needed. This change removes `invocationLocation` in favor of `invocationStrategy` and changes the internal representation of `InvocationStrategy::Once` to hold the workspace root.
Closes#17848.
These flags were added to help rust-analyzer integrate with repos
requiring non-Cargo invocations. The consensus is that having two
independent settings are no longer needed. This change removes
`invocationLocation` in favor of `invocationStrategy` and changes
the internal representation of `InvocationStrategy::Once` to hold
the workspace root.
fix: Wrong BoundVar index when lowering impl trait parameter of parent generics
Fixes#17711
From the following test code;
```rust
//- minicore: deref
use core::ops::Deref;
struct Struct<'a, T>(&'a T);
trait Trait {}
impl<'a, T: Deref<Target = impl Trait>> Struct<'a, T> {
fn foo(&self) -> &Self { self }
fn bar(&self) {
let _ = self.foo();
}
}
```
when we call `register_obligations_for_call` for `let _ = self.foo();`,
07659783fd/crates/hir-ty/src/infer/expr.rs (L1939-L1952)
we are querying `generic_predicates` and it has `T: Deref<Target = impl Trait>` predicate from the parent `impl Struct`;
07659783fd/crates/hir-ty/src/lower.rs (L375-L399)
but as we can see above, lowering `TypeRef = impl Trait` doesn't take into account the parent generic parameters, so the `BoundVar` index here is `0`, as `fn foo` has no generic args other than parent's,
But this `BoundVar` is pointing at `'a` in `<'a, T: Deref<Target = impl Trait>>`.
So, in the first code reference `register_obligations_for_call`'s L:1948 - `.substitute(Interner, parameters)`, we are substituting `'a` with `Ty`, not `Lifetime` and this makes panic inside the chalk.
This PR fixes this wrong `BoundVar` index in such cases
Remove rust-analyzer.workspace.discoverProjectRunner
The functionality for this vscode config option was removed in #17395, so it doesn't do anything anymore.
Add scip/lsif flag to exclude vendored libaries
#17809 changed StaticIndex to include vendored libraries. This PR adds a flag to disable that behavior.
At work, our monorepo has too many rust targets to index all at once, so we split them up into several shards. Since all of our libraries are vendored, if rust-analyzer includes them, sharding no longer has much benefit, because every shard will have to index the entire transitive dependency graphs of all of its targets. We get around the issue presented in #17809 because some other shard will index the libraries directly.
fix: Properly account for editions in names
This PR touches a lot of parts. But the main changes are changing `hir_expand::Name` to be raw edition-dependently and only when necessary (unrelated to how the user originally wrote the identifier), and changing `is_keyword()` and `is_raw_identifier()` to be edition-aware (this was done in #17896, but the FIXMEs were fixed here).
It is possible that I missed some cases, but most IDE parts should properly escape (or not escape) identifiers now.
The rules of thumb are:
- If we show the identifier to the user, its rawness should be determined by the edition of the edited crate. This is nice for IDE features, but really important for changes we insert to the source code.
- For tests, I chose `Edition::CURRENT` (so we only have to (maybe) update tests when an edition becomes stable, to avoid churn).
- For debugging tools (helper methods and logs), I used `Edition::LATEST`.
Reviewing notes:
This is a really big PR but most of it is mechanical translation. I changed `Name` displayers to require an edition, and followed the compiler errors. Most methods just propagate the edition requirement. The interesting cases are mostly in `ide-assists`, as sometimes the correct crate to fetch the edition from requires awareness (there may be two). `ide-completions` and `ide-diagnostics` were solved pretty easily by introducing an edition field to their context. `ide` contains many features, for most of them it was propagated to the top level function and there the edition was fetched based on the file.
I also fixed all FIXMEs from #17896. Some required introducing an edition parameter (usually not for many methods after the changes to `Name`), some were changed to a new method `is_any_identifier()` because they really want any possible keyword.
Fixes#17895.
Fixes#17774.
This PR touches a lot of parts. But the main changes are changing
`hir_expand::Name` to be raw edition-dependently and only when necessary
(unrelated to how the user originally wrote the identifier),
and changing `is_keyword()` and `is_raw_identifier()` to be edition-aware
(this was done in #17896, but the FIXMEs were fixed here).
It is possible that I missed some cases, but most IDE parts should properly
escape (or not escape) identifiers now.
The rules of thumb are:
- If we show the identifier to the user, its rawness should be determined
by the edition of the edited crate. This is nice for IDE features,
but really important for changes we insert to the source code.
- For tests, I chose `Edition::CURRENT` (so we only have to (maybe) update
tests when an edition becomes stable, to avoid churn).
- For debugging tools (helper methods and logs), I used `Edition::LATEST`.
Allow flycheck process to exit gracefully
Assuming it isn't cancelled. Closes#17902.
The only place CommandHandle::join() is used is when the flycheck command
finishes, so this commit changes the behavior of the method itself.
The only reason I can see for the existing behavior is if the command is somehow holding onto a build lock longer than it should, this would force it to be released. But it would be a pretty heavy-handed way to solve that issue. I'm not aware of this occurring in practice.
Test for word boundary in `FindUsages`
This speeds up short identifiers search significantly, while unlikely to have an effect on long identifiers (the analysis takes much longer than some character comparison).
Tested by finding all references to `eq()` (from `PartialEq`) in the rust-analyzer repo. Total time went down from 100s to 10s (a 10x reduction!).
Feel free to close this if you consider this a non-issue, as most short identifiers are local.
internal: Replace once_cell with std's recently stabilized OnceCell/Lock and LazyCell/Lock
This doesn't get rid of the once_cell dependency, unfortunately, since we have dependencies that use it, but it's a nice to do cleanup. And when our deps will eventually get rid of once_cell we will get rid of it for free.
This speeds up short identifiers search significantly, while unlikely to have an effect on long identifiers (the analysis takes much longer than some character comparison).
Tested by finding all references to `eq()` (from `PartialEq`) in the rust-analyzer repo. Total time went down from 100s to 10s (a 10x reduction!).
This doesn't get rid of the once_cell dependency, unfortunately, since we have dependencies that use it, but it's a nice to do cleanup. And when our deps will eventually get rid of once_cell we will get rid of it for free.
Assuming it isn't cancelled. Closes#17902.
The only place CommandHandle::join is used is when the flycheck command
finishes, so this commit changes the behavior of the method itself.
internal: Properly check the edition for edition dependent syntax kinds
This puts the current edition in a bunch of places, most of which I annoted with FIXMEs asside from the ones in ide-assists because I couldnt bother with those
fix: Panic while hovering associated function with type annotation on generic param that not inherited from its container type
Fixes#17871
We call `generic_args_sans_defaults` here;
64a140527b/crates/hir-ty/src/display.rs (L1021-L1034)
but the following substitution inside that function panic in #17871;
64a140527b/crates/hir-ty/src/display.rs (L1468)
it's because the `Binders.binder` inside `default_parameters` has a same length with the generics of the function we are hovering on, but the generics of it is split into two, `fn_params` and `parent_params`.
Because of this, it may panic if the function has one or more default parameters and both `fn_params` and `parent_params` are non-empty, like the case in the title of this PR.
So, we must call `generic_args_sans_default` first and then split it into `fn_params` and `parent_params`
fix: Panic while canonicalizing erroneous projection type
Fixes#17866
The root cause of #17866 is quite horrifyng 😨
```rust
trait T {
type A;
}
type Foo = <S as T>::A; // note that S isn't defined
fn main() {
Foo {}
}
```
While inferencing alias type `Foo = <S as T>::A`;
78c2bdce86/crates/hir-ty/src/infer.rs (L1388-L1398)
the error type `S` in it is substituted by inference var in L1396 above as below;
78c2bdce86/crates/hir-ty/src/infer/unify.rs (L866-L869)
This new inference var's index is `1`, as the type inferecing procedure here previously inserted another inference var into same `InferenceTable`.
But after that, the projection type made from the above then passed to the following function;
78c2bdce86/crates/hir-ty/src/traits.rs (L88-L96)
here, a whole new `InferenceTable` is made, without any inference var and in the L94, this table calls;
78c2bdce86/crates/hir-ty/src/infer/unify.rs (L364-L370)
And while registering `AliasEq` `obligation`, this obligation contains inference var `?1` made from the previous table, but this table has only one inference var `?0` made at L365.
So, the chalk panics when we try to canonicalize that obligation to register it, because the obligation contains an inference var `?1` that the canonicalizing table doesn't have.
Currently, we are calling `InferenceTable::new()` to do some normalizing, unifying or coercing things to some targets that might contain inference var that the new table doesn't have.
I think that this is quite dangerous footgun because the inference var is just an index that does not contain the information which table does it made from, so sometimes this "foreign" index might cause panic like this case, or point at the wrong variable.
This PR mitigates such behaviour simply by inserting sufficient number of inference vars to new table to avoid such problem.
This strategy doesn't harm current r-a's intention because the inference vars that passed into new tables are just "unresolved" variables in current r-a, so this is just making sure that such "unresolved" variables exist in the new table
internal: Be more resilient to bad language item definitions in binop inference
Fixes#16287Fixes#16286
There's one more in `write_fn_trait_method_resolution`, but I'm not sure if it won't cause further problems in `infer_closures`.
Fix blessing of rmake tests
Fixes#129038.
When running in `--bless` mode, we now set the value of `RUSTC_BLESS_TEST` to the current test's source directory. This allows the diff helper in `run_make_support` to find the original snapshot file in the source directory and bless that, instead of unhelpfully blessing the temporary copy in `build`.
r? `@jieyouxu`
CFI: Move CFI ui tests to cfi directory
Move the CFI ui tests to the cfi directory and removes the cfi prefix from tests file names similarly to how the cfi codegen tests are organized.
Slightly refactor `TargetSelection` in bootstrap
Mostly a drive-by refactoring of `TargetSelection` to reduce some manual "windows-gnu" detection and also accesses to the `triple` field.
r? `@onur-ozkan`
bootstrap: don't use rustflags for `--rustc-args`
r? `@onur-ozkan`
This is going to require a bit of context.
https://github.com/rust-lang/rust/pull/47558 has added `--rustc-args` to `./x test` to allow passing flags when building `compiletest` tests. It was made specifically because using `RUSTFLAGS` would rebuild the compiler/stdlib, which would in turn require the flag you want to build tests with to successfully bootstrap.
#113178 made the request that it also works for other tests and doctests. This is not trivial to support across the board for `library`/`compiler` unit-tests/doctests and across stages. This issue was closed in #113948 by using `RUSTFLAGS`, seemingly incorrectly since https://github.com/rust-lang/rust/pull/123489 fixed that part to make it work.
Unfortunately #123489/#113948 have regressed the goals of `--rustc-args`:
- now we can't use rustc args that don't bootstrap, to run the UI tests: we can't test incomplete features. The new trait solver doesn't bootstrap, in-progress borrowck/polonius changes don't bootstrap, some other features are similarly incomplete, etc.
- using the flag now rebuilds everything from scratch: stage0 stdlib, stage1 compiler, stage1 stdlib. You don't need to re-do all this to compile UI tests, you only need the latter to run stdlib tests with a new flag, etc. This happens for contributors, but also on CI today. (Not to mention that in doing that it will rebuild things with flags that are not meant to be used, e.g. stdlib cfgs that don't exist in the compiler; or you could also imagine that this silently enables flags that were not meant to be enabled in this way).
Since then, bd71c71ea0 has started using it to test a stdlib feature, relying on the fact that it now rebuilds everything. So #125011 also regressed CI times more than necessary because it rebuilds everything instead of just stage 1 stdlib.
It's not easy for me to know how to properly fix#113178 in bootstrap, but #113948/#123489 are not it since they regress the initial intent. I'd think bootstrap would have to know from the list of test targets that are passed that the `library` or `compiler` paths that are passed could require rebuilding these crates with different rustflags, probably also depending on stages. Therefore I would not be able to fix it, and will just try in this PR to unregress the situation to unblock the initial use-case.
It seems miri now also uses `./x miri --rustc-args` in this incorrect meaning to rebuild the `library` paths they support to run with the new args. I've not made any bootstrap changes related to `./x miri` in this PR, so `--rustc-args` wouldn't work there anymore. I'd assume this would need to use rustflags again but I don't know how to make that work properly in bootstrap, hence opening as draft, so you can tell me how to do that. I assume we don't want to break their use-case again now that it exists, even though there are ways to use `./x test` to do exactly that.
`RUSTFLAGS_NOT_BOOTSTRAP=flag ./x test library/std` is a way to run unit tests with a new flag without rebuilding everything, while with #123489 there is no way anymore to run tests with a flag that doesn't bootstrap.
---
edit: after review, this PR:
- renames `./x test --rustc-args` to `./x test --compiletest-rustc-args` as it only applies there, and cannot use rustflags for this purpose.
- fixes the regression that using these args rebuilt everything from scratch
- speeds up some CI jobs via the above point
- removes `./x miri --rustc-args` as only library tests are supported, needs to rebuild libstd, and `./x miri --compiletest-rustc-args` wouldn't work since compiletests are not supported.
