[PATCH v3] scripts: generate_rust_analyzer.py: add missing macros deps

Tamir Duberstein posted 1 patch 12 months ago
scripts/generate_rust_analyzer.py | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
[PATCH v3] scripts: generate_rust_analyzer.py: add missing macros deps
Posted by Tamir Duberstein 12 months ago
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>
Re: [PATCH v3] scripts: generate_rust_analyzer.py: add missing macros deps
Posted by Miguel Ojeda 11 months ago
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
Re: [PATCH v3] scripts: generate_rust_analyzer.py: add missing macros deps
Posted by Andreas Hindborg 11 months, 1 week ago
"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
Re: [PATCH v3] scripts: generate_rust_analyzer.py: add missing macros deps
Posted by Tamir Duberstein 11 months, 1 week ago
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!
Re: [PATCH v3] scripts: generate_rust_analyzer.py: add missing macros deps
Posted by Miguel Ojeda 11 months, 1 week ago
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
Re: [PATCH v3] scripts: generate_rust_analyzer.py: add missing macros deps
Posted by Tamir Duberstein 11 months, 1 week ago
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.
Re: [PATCH v3] scripts: generate_rust_analyzer.py: add missing macros deps
Posted by Tamir Duberstein 11 months ago
Hi Miguel, gentle ping on this. Please let me know what, if anything,
is required to move this forward.

Cheers.
Tamir
Re: [PATCH v3] scripts: generate_rust_analyzer.py: add missing macros deps
Posted by Miguel Ojeda 11 months, 1 week ago
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
Re: [PATCH v3] scripts: generate_rust_analyzer.py: add missing macros deps
Posted by Tamir Duberstein 11 months, 1 week ago
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