scripts/generate_rust_analyzer.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-)
The macros crate has depended on std and proc_macro since its
introduction in commit 1fbde52bde73 ("rust: add `macros` crate"). These
dependencies were omitted from commit 8c4555ccc55c ("scripts: add
`generate_rust_analyzer.py`") resulting in missing go-to-definition and
autocomplete, and false-positive warnings emitted from rust-analyzer
such as:
[{
"resource": "/Users/tamird/src/linux/rust/macros/module.rs",
"owner": "_generated_diagnostic_collection_name_#1",
"code": {
"value": "non_snake_case",
"target": {
"$mid": 1,
"path": "/rustc/",
"scheme": "https",
"authority": "doc.rust-lang.org",
"query": "search=non_snake_case"
}
},
"severity": 4,
"message": "Variable `None` should have snake_case name, e.g. `none`",
"source": "rust-analyzer",
"startLineNumber": 123,
"startColumn": 17,
"endLineNumber": 123,
"endColumn": 21
}]
Add the missing dependencies to improve the developer experience.
Fixes: 8c4555ccc55c ("scripts: add `generate_rust_analyzer.py`")
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Suggested-by: Chayim Refael Friedman <chayimfr@gmail.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v3:
- Avoid shuffling compilter_builtins; it is not needed for RA support.
- Align more closely with the long-term solution:
https://lore.kernel.org/all/20250209-rust-analyzer-host-v1-0-a2286a2a2fa3@gmail.com/.
- Link to v2: https://lore.kernel.org/r/20250209-rust-analyzer-macros-core-dep-v2-1-897338344d16@gmail.com
Changes in v2:
- Change macros deps from [core] to [std, proc_macro], improving
autocomplete and go-to-definition.
- Remove Wedson Almeida Filho <wedsonaf@google.com> from cc; email
bounced.
- Link to v1: https://lore.kernel.org/r/20250209-rust-analyzer-macros-core-dep-v1-1-5ebeb3eb60a9@gmail.com
---
scripts/generate_rust_analyzer.py | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index aa8ea1a4dbe5..1394baa5ee9e 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -57,14 +57,26 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
crates_indexes[display_name] = len(crates)
crates.append(crate)
- # First, the ones in `rust/` since they are a bit special.
- append_crate(
- "core",
- sysroot_src / "core" / "src" / "lib.rs",
- [],
- cfg=crates_cfgs.get("core", []),
- is_workspace_member=False,
- )
+ def append_sysroot_crate(
+ display_name,
+ deps,
+ cfg=[],
+ ):
+ return append_crate(
+ display_name,
+ sysroot_src / display_name / "src" / "lib.rs",
+ deps,
+ cfg,
+ is_workspace_member=False,
+ )
+
+ # NB: sysroot crates reexport items from one another so setting up our transitive dependencies
+ # here is important for ensuring that rust-analyzer can resolve symbols. The sources of truth
+ # for this dependency graph are `(sysroot_src / crate / "Cargo.toml" for crate in crates)`.
+ append_sysroot_crate("core", [], cfg=crates_cfgs.get("core", []))
+ append_sysroot_crate("alloc", ["core"])
+ append_sysroot_crate("std", ["alloc", "core"])
+ append_sysroot_crate("proc_macro", ["core", "std"])
append_crate(
"compiler_builtins",
@@ -75,7 +87,7 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
append_crate(
"macros",
srctree / "rust" / "macros" / "lib.rs",
- [],
+ ["std", "proc_macro"],
is_proc_macro=True,
)
---
base-commit: 6273a058383e05465083b535ed9469f2c8a48321
change-id: 20250209-rust-analyzer-macros-core-dep-0f57868dd19f
Best regards,
--
Tamir Duberstein <tamird@gmail.com>On Mon, Feb 10, 2025 at 6:03 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> The macros crate has depended on std and proc_macro since its
> introduction in commit 1fbde52bde73 ("rust: add `macros` crate"). These
> dependencies were omitted from commit 8c4555ccc55c ("scripts: add
> `generate_rust_analyzer.py`") resulting in missing go-to-definition and
> autocomplete, and false-positive warnings emitted from rust-analyzer
> such as:
>
> [{
> "resource": "/Users/tamird/src/linux/rust/macros/module.rs",
> "owner": "_generated_diagnostic_collection_name_#1",
> "code": {
> "value": "non_snake_case",
> "target": {
> "$mid": 1,
> "path": "/rustc/",
> "scheme": "https",
> "authority": "doc.rust-lang.org",
> "query": "search=non_snake_case"
> }
> },
> "severity": 4,
> "message": "Variable `None` should have snake_case name, e.g. `none`",
> "source": "rust-analyzer",
> "startLineNumber": 123,
> "startColumn": 17,
> "endLineNumber": 123,
> "endColumn": 21
> }]
>
> Add the missing dependencies to improve the developer experience.
>
> Fixes: 8c4555ccc55c ("scripts: add `generate_rust_analyzer.py`")
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Suggested-by: Chayim Refael Friedman <chayimfr@gmail.com>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Applied to `rust-fixes` -- thanks everyone!
[ Fiona had a different approach (thanks!) at:
https://lore.kernel.org/rust-for-linux/20241205115438.234221-1-me@kloenk.dev/
But Tamir and Fiona agreed to this one. - Miguel ]
[ Removed `return`. Changed tag name. Added Link. Slightly
reworded. - Miguel ]
Cheers,
Miguel
"Tamir Duberstein" <tamird@gmail.com> writes:
> The macros crate has depended on std and proc_macro since its
> introduction in commit 1fbde52bde73 ("rust: add `macros` crate"). These
> dependencies were omitted from commit 8c4555ccc55c ("scripts: add
> `generate_rust_analyzer.py`") resulting in missing go-to-definition and
> autocomplete, and false-positive warnings emitted from rust-analyzer
> such as:
>
> [{
> "resource": "/Users/tamird/src/linux/rust/macros/module.rs",
> "owner": "_generated_diagnostic_collection_name_#1",
> "code": {
> "value": "non_snake_case",
> "target": {
> "$mid": 1,
> "path": "/rustc/",
> "scheme": "https",
> "authority": "doc.rust-lang.org",
> "query": "search=non_snake_case"
> }
> },
> "severity": 4,
> "message": "Variable `None` should have snake_case name, e.g. `none`",
> "source": "rust-analyzer",
> "startLineNumber": 123,
> "startColumn": 17,
> "endLineNumber": 123,
> "endColumn": 21
> }]
>
> Add the missing dependencies to improve the developer experience.
>
> Fixes: 8c4555ccc55c ("scripts: add `generate_rust_analyzer.py`")
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Suggested-by: Chayim Refael Friedman <chayimfr@gmail.com>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> Changes in v3:
> - Avoid shuffling compilter_builtins; it is not needed for RA support.
> - Align more closely with the long-term solution:
> https://lore.kernel.org/all/20250209-rust-analyzer-host-v1-0-a2286a2a2fa3@gmail.com/.
> - Link to v2: https://lore.kernel.org/r/20250209-rust-analyzer-macros-core-dep-v2-1-897338344d16@gmail.com
>
> Changes in v2:
> - Change macros deps from [core] to [std, proc_macro], improving
> autocomplete and go-to-definition.
> - Remove Wedson Almeida Filho <wedsonaf@google.com> from cc; email
> bounced.
> - Link to v1: https://lore.kernel.org/r/20250209-rust-analyzer-macros-core-dep-v1-1-5ebeb3eb60a9@gmail.com
> ---
> scripts/generate_rust_analyzer.py | 30 +++++++++++++++++++++---------
> 1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
> index aa8ea1a4dbe5..1394baa5ee9e 100755
> --- a/scripts/generate_rust_analyzer.py
> +++ b/scripts/generate_rust_analyzer.py
> @@ -57,14 +57,26 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> crates_indexes[display_name] = len(crates)
> crates.append(crate)
>
> - # First, the ones in `rust/` since they are a bit special.
> - append_crate(
> - "core",
> - sysroot_src / "core" / "src" / "lib.rs",
> - [],
> - cfg=crates_cfgs.get("core", []),
> - is_workspace_member=False,
> - )
> + def append_sysroot_crate(
> + display_name,
> + deps,
> + cfg=[],
> + ):
> + return append_crate(
Why the `return` here?
Otherwise looks good to me.
I never had any errors as described above, but I can confirm that go to
definition in `macros` for items in `proc_macro` work with this patch
applied. It does not work prior to applying the patch.
Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
Best regards,
Andreas Hindborg
On Fri, Mar 7, 2025 at 5:57 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Tamir Duberstein" <tamird@gmail.com> writes:
>
> > The macros crate has depended on std and proc_macro since its
> > introduction in commit 1fbde52bde73 ("rust: add `macros` crate"). These
> > dependencies were omitted from commit 8c4555ccc55c ("scripts: add
> > `generate_rust_analyzer.py`") resulting in missing go-to-definition and
> > autocomplete, and false-positive warnings emitted from rust-analyzer
> > such as:
> >
> > [{
> > "resource": "/Users/tamird/src/linux/rust/macros/module.rs",
> > "owner": "_generated_diagnostic_collection_name_#1",
> > "code": {
> > "value": "non_snake_case",
> > "target": {
> > "$mid": 1,
> > "path": "/rustc/",
> > "scheme": "https",
> > "authority": "doc.rust-lang.org",
> > "query": "search=non_snake_case"
> > }
> > },
> > "severity": 4,
> > "message": "Variable `None` should have snake_case name, e.g. `none`",
> > "source": "rust-analyzer",
> > "startLineNumber": 123,
> > "startColumn": 17,
> > "endLineNumber": 123,
> > "endColumn": 21
> > }]
> >
> > Add the missing dependencies to improve the developer experience.
> >
> > Fixes: 8c4555ccc55c ("scripts: add `generate_rust_analyzer.py`")
> > Reviewed-by: Fiona Behrens <me@kloenk.dev>
> > Suggested-by: Chayim Refael Friedman <chayimfr@gmail.com>
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> > Changes in v3:
> > - Avoid shuffling compilter_builtins; it is not needed for RA support.
> > - Align more closely with the long-term solution:
> > https://lore.kernel.org/all/20250209-rust-analyzer-host-v1-0-a2286a2a2fa3@gmail.com/.
> > - Link to v2: https://lore.kernel.org/r/20250209-rust-analyzer-macros-core-dep-v2-1-897338344d16@gmail.com
> >
> > Changes in v2:
> > - Change macros deps from [core] to [std, proc_macro], improving
> > autocomplete and go-to-definition.
> > - Remove Wedson Almeida Filho <wedsonaf@google.com> from cc; email
> > bounced.
> > - Link to v1: https://lore.kernel.org/r/20250209-rust-analyzer-macros-core-dep-v1-1-5ebeb3eb60a9@gmail.com
> > ---
> > scripts/generate_rust_analyzer.py | 30 +++++++++++++++++++++---------
> > 1 file changed, 21 insertions(+), 9 deletions(-)
> >
> > diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
> > index aa8ea1a4dbe5..1394baa5ee9e 100755
> > --- a/scripts/generate_rust_analyzer.py
> > +++ b/scripts/generate_rust_analyzer.py
> > @@ -57,14 +57,26 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> > crates_indexes[display_name] = len(crates)
> > crates.append(crate)
> >
> > - # First, the ones in `rust/` since they are a bit special.
> > - append_crate(
> > - "core",
> > - sysroot_src / "core" / "src" / "lib.rs",
> > - [],
> > - cfg=crates_cfgs.get("core", []),
> > - is_workspace_member=False,
> > - )
> > + def append_sysroot_crate(
> > + display_name,
> > + deps,
> > + cfg=[],
> > + ):
> > + return append_crate(
>
> Why the `return` here?
It's in anticipation of this follow up patch
https://lore.kernel.org/all/20250209-rust-analyzer-host-v1-2-a2286a2a2fa3@gmail.com/
where the return value is used to identify the crate. I developed the
two in parallel so I added this return to reduce overall churn.
> Otherwise looks good to me.
>
> I never had any errors as described above, but I can confirm that go to
> definition in `macros` for items in `proc_macro` work with this patch
> applied. It does not work prior to applying the patch.
>
>
> Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
Thanks!
On Fri, Mar 7, 2025 at 12:06 PM Tamir Duberstein <tamird@gmail.com> wrote: > > It's in anticipation of this follow up patch > > https://lore.kernel.org/all/20250209-rust-analyzer-host-v1-2-a2286a2a2fa3@gmail.com/ > > where the return value is used to identify the crate. I developed the > two in parallel so I added this return to reduce overall churn. For this case, it is not a big deal (and sometimes it is a good idea, e.g. a `return` in a public API that wouldn't make sense not to have), but in general, please avoid doing things to avoid churn across series if that can confuse others -- future patches may or may not end up getting merged, so it is best not to rely on the future. Thanks! Cheers, Miguel
On Fri, Mar 7, 2025 at 6:48 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Fri, Mar 7, 2025 at 12:06 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > It's in anticipation of this follow up patch > > > > https://lore.kernel.org/all/20250209-rust-analyzer-host-v1-2-a2286a2a2fa3@gmail.com/ > > > > where the return value is used to identify the crate. I developed the > > two in parallel so I added this return to reduce overall churn. > > For this case, it is not a big deal (and sometimes it is a good idea, > e.g. a `return` in a public API that wouldn't make sense not to have), > but in general, please avoid doing things to avoid churn across series > if that can confuse others -- future patches may or may not end up > getting merged, so it is best not to rely on the future. Makes sense, thanks.
Hi Miguel, gentle ping on this. Please let me know what, if anything, is required to move this forward. Cheers. Tamir
On Mon, Feb 10, 2025 at 6:03 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> The macros crate has depended on std and proc_macro since its
> introduction in commit 1fbde52bde73 ("rust: add `macros` crate"). These
> dependencies were omitted from commit 8c4555ccc55c ("scripts: add
> `generate_rust_analyzer.py`") resulting in missing go-to-definition and
> autocomplete, and false-positive warnings emitted from rust-analyzer
> such as:
>
> [{
> "resource": "/Users/tamird/src/linux/rust/macros/module.rs",
> "owner": "_generated_diagnostic_collection_name_#1",
> "code": {
> "value": "non_snake_case",
> "target": {
> "$mid": 1,
> "path": "/rustc/",
> "scheme": "https",
> "authority": "doc.rust-lang.org",
> "query": "search=non_snake_case"
> }
> },
> "severity": 4,
> "message": "Variable `None` should have snake_case name, e.g. `none`",
> "source": "rust-analyzer",
> "startLineNumber": 123,
> "startColumn": 17,
> "endLineNumber": 123,
> "endColumn": 21
> }]
>
> Add the missing dependencies to improve the developer experience.
>
> Fixes: 8c4555ccc55c ("scripts: add `generate_rust_analyzer.py`")
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Suggested-by: Chayim Refael Friedman <chayimfr@gmail.com>
If this is a fix, then it should use Reported-by -- or the suggestion
was about the implementation?
Anyway, I can fix it on my side, but I am also supposed to add a link
to the report/suggestion -- do you have one?
Thanks!
(Tested-by's welcome, by the way!)
Cheers,
Miguel
On Wed, Mar 5, 2025 at 6:14 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Mon, Feb 10, 2025 at 6:03 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > The macros crate has depended on std and proc_macro since its
> > introduction in commit 1fbde52bde73 ("rust: add `macros` crate"). These
> > dependencies were omitted from commit 8c4555ccc55c ("scripts: add
> > `generate_rust_analyzer.py`") resulting in missing go-to-definition and
> > autocomplete, and false-positive warnings emitted from rust-analyzer
> > such as:
> >
> > [{
> > "resource": "/Users/tamird/src/linux/rust/macros/module.rs",
> > "owner": "_generated_diagnostic_collection_name_#1",
> > "code": {
> > "value": "non_snake_case",
> > "target": {
> > "$mid": 1,
> > "path": "/rustc/",
> > "scheme": "https",
> > "authority": "doc.rust-lang.org",
> > "query": "search=non_snake_case"
> > }
> > },
> > "severity": 4,
> > "message": "Variable `None` should have snake_case name, e.g. `none`",
> > "source": "rust-analyzer",
> > "startLineNumber": 123,
> > "startColumn": 17,
> > "endLineNumber": 123,
> > "endColumn": 21
> > }]
> >
> > Add the missing dependencies to improve the developer experience.
> >
> > Fixes: 8c4555ccc55c ("scripts: add `generate_rust_analyzer.py`")
> > Reviewed-by: Fiona Behrens <me@kloenk.dev>
> > Suggested-by: Chayim Refael Friedman <chayimfr@gmail.com>
>
> If this is a fix, then it should use Reported-by -- or the suggestion
> was about the implementation?
The most correct thing here would be Diagnosed-by 😅
> Anyway, I can fix it on my side, but I am also supposed to add a link
> to the report/suggestion -- do you have one?
https://github.com/rust-lang/rust-analyzer/issues/17759#issuecomment-2646328275
> Thanks!
>
> (Tested-by's welcome, by the way!)
>
> Cheers,
> Miguel
Thanks!
Tamir
© 2016 - 2025 Red Hat, Inc.