Avoid unused tracing dep attributes feature in syntax/parser crates

[lmmx]

I'm using the ra_ap_syntax crate published from the crates/syntax dir in this repo and would like if it could feature-gate the tracing instrumentation (so I can avoid the dependencies it induces). This would reduce the crate tree size considerably.

I noticed the parser crate appeared to already do so:

rust-analyzer/crates/parser/Cargo.toml

Line 30 in 5062185

default = ["tracing"]

but then it seems to have been unfinished, the feature was never used in the code:

rust-analyzer/crates/parser/src/lib.rs

Line 92 in 5062185

let _p = tracing::info_span!("TopEntryPoint::parse", ?self).entered();

As such the parser crate did not compile when the tracing feature is configured out (--no-default-features).

louis 🚶 ~/lab/rust-analyzer/crates/parser $ cargo check --no-default-features
    Checking parser v0.0.0 (/home/louis/lab/rust-analyzer/crates/parser)
error[E0433]: failed to resolve: use of unresolved module or unlinked crate `tracing`
  --> crates/parser/src/lib.rs:92:18
   |
92 |         let _p = tracing::info_span!("TopEntryPoint::parse", ?self).entered();
   |                  ^^^^^^^ use of unresolved module or unlinked crate `tracing`

error[E0433]: failed to resolve: use of unresolved module or unlinked crate `tracing`
  --> crates/parser/src/shortcuts.rs:29:18
   |
29 |         let _p = tracing::info_span!("LexedStr::to_input").entered();
   |                  ^^^^^^^ use of unresolved module or unlinked crate `tracing`

error[E0433]: failed to resolve: use of unresolved module or unlinked crate `tracing`
  --> crates/parser/src/lexed_str.rs:38:18
   |
38 |         let _p = tracing::info_span!("LexedStr::new").entered();
   |                  ^^^^^^^ use of unresolved module or unlinked crate `tracing`

• Fixed in PR fix(parser): feature gate tracing spans #21108 for parser crate and updated syntax crate with a new tracing feature to do likewise

[ChayimFriedman2]

Hmm... I don't like complicating the code and tracing is freely used in the rest of r-a, however those crates do stand on their own and can be useful... @rust-lang/rust-analyzer what do you think?

[davidbarsky]

I think I’m quite conflicted about this (from a maintainership perspective), but tracing shouldn’t have many dependencies to be begin with.

[lmmx]

Hmm... I don't like complicating the code and tracing is freely used in the rest of r-a

To clarify, this change wouldn't impact the rest of r-a: it only affects builds of the parsing/syntax crates with --no-default-features, which is already supported but currently broken.

The parser crate already declares default = ["tracing"] in its Cargo.toml, suggesting this was the intended design. The PR just wires it up correctly with 3 lines of cfg guards, and 5 in syntax crate: #21108

I think I’m quite conflicted about this (from a maintainership perspective)

What's making you conflicted about it David? It looks like this was actually already meant to be implemented this way right?

rust-analyzer/crates/parser/Cargo.toml

Line 30 in 5062185

default = ["tracing"]

Is there something I am not seeing here about why this would be an issue to maintain?

but tracing shouldn’t have many dependencies to be begin with.

Unfortunately tracing dep is specified in the parser crate as just tracing.workspace = true and the workspace dep is listed without default-features = false

rust-analyzer/crates/syntax/Cargo.toml

Line 23 in 8c978e3

tracing.workspace = true

hence this pulls in the default features for tracing, which includes tracing-attributes (via tokio-rs/tracing/Cargo.toml#L43)

[features]
default = ["std", "attributes"]

hence this pulls in syn and proc macro 2 deps (see here) which must therefore be built for every project using ra_ap_syntax even if they're not making use of the tracing functionality.

• More specifically, we build the proc macro and syn dependencies for parsing the entire Rust AST here for an attributes feature that is not used: the #[instrument] attribute macro is not used by either parser/syntax to inject tracing spans. They create their spans as direct tracing::info_span! invocations (i.e. they do not use an attribute macro so do not need the attribute feature, it's just being pulled in as a default feature).

rust-analyzer/Cargo.toml

Line 159 in 8c978e3

tracing = "0.1.41"

louis 🚶 ~/lab/rust-analyzer/crates/parser $ cargo check
   Compiling proc-macro2 v1.0.101
   Compiling unicode-ident v1.0.19
    Checking once_cell v1.21.3
    Checking pin-project-lite v0.2.16
    Checking memchr v2.7.6
    Checking unicode-xid v0.2.6
    Checking unicode-properties v0.1.3
    Checking tracing-core v0.1.34
    Checking drop_bomb v0.1.5
    Checking edition v0.0.0 (/home/louis/lab/rust-analyzer/crates/edition)
    Checking rustc-literal-escaper v0.0.4
    Checking winnow v0.7.13
    Checking ra-ap-rustc_lexer v0.137.0
   Compiling quote v1.0.40
   Compiling syn v2.0.106
   Compiling tracing-attributes v0.1.30
    Checking tracing v0.1.41
    Checking parser v0.0.0 (/home/louis/lab/rust-analyzer/crates/parser)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 19.63s

• Cargo clean reports 45 MB total

If I instead go about this by turning off default features (I think this may have just been an oversight, the default features for tracing include attributes, which this crate does not use the #[instrument] attribute macro) then you would achieve the same result a different way:

louis 🚶 ~/lab/rust-analyzer/crates/parser $ cargo check
    Checking pin-project-lite v0.2.16
    Checking memchr v2.7.6
    Checking unicode-xid v0.2.6
    Checking tracing-core v0.1.34
    Checking unicode-properties v0.1.3
    Checking rustc-literal-escaper v0.0.4
    Checking winnow v0.7.13
    Checking edition v0.0.0 (/home/louis/lab/rust-analyzer/crates/edition)
    Checking drop_bomb v0.1.5
    Checking tracing v0.1.41
    Checking ra-ap-rustc_lexer v0.137.0
    Checking parser v0.0.0 (/home/louis/lab/rust-analyzer/crates/parser)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 3.24s

• 6x faster to compile than with the unused tracing/attributes feature activated
• Cargo clean reports 12 MB

The #[instrument] attribute is used in crates project-model and hir-ty only:

louis 🚶 ~/lab/rust-analyzer $ rg '#\[instrument' -l
crates/project-model/src/workspace.rs
crates/hir-ty/src/infer/opaques.rs
...
crates/hir-ty/src/next_solver/infer/opaque_types/table.rs

So an alternative route to the same outcome is to change the workspace-level tracing default to false, set the features to just ["std"] and set them to ["std", "attributes"] only for the project-model and hir-ty crates, rather than feature gating the syntax/parser crates' invocation of the tracing spans altogether.

I just tried this and confirmed it works

Click to show git diff of this alternative

diff --git a/Cargo.toml b/Cargo.toml
index a7f1e17..63223f8 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -156,7 +156,7 @@ smol_str = "0.3.2"
 temp-dir = "0.1.16"
 text-size = "1.1.1"
 toml = "0.9.8"
-tracing = "0.1.41"
+tracing = { version = "0.1.41", default-features = false, features = ["std"] }
 tracing-tree = "0.4.0"
 tracing-subscriber = { version = "0.3.20", default-features = false, features = [
     "registry",
diff --git a/crates/hir-ty/Cargo.toml b/crates/hir-ty/Cargo.toml
index 378a0f0..39769b2 100644
--- a/crates/hir-ty/Cargo.toml
+++ b/crates/hir-ty/Cargo.toml
@@ -21,7 +21,7 @@ smallvec.workspace = true
 ena = "0.14.3"
 either.workspace = true
 oorandom = "11.1.5"
-tracing.workspace = true
+tracing = { workspace = true, features = ["std", "attributes"] }
 rustc-hash.workspace = true
 scoped-tls = "1.0.1"
 la-arena.workspace = true
diff --git a/crates/project-model/Cargo.toml b/crates/project-model/Cargo.toml
index ec44369..e00e8de 100644
--- a/crates/project-model/Cargo.toml
+++ b/crates/project-model/Cargo.toml
@@ -22,7 +22,7 @@ serde.workspace = true
 serde_derive.workspace = true
 temp-dir.workspace = true
 toml.workspace = true
-tracing.workspace = true
+tracing = { workspace = true, features = ["std", "attributes"] }
 triomphe.workspace = true
 la-arena.workspace = true
 itertools.workspace = true

I see #21108 as a preferable solution though as it configures out the tracing dep entirely so that there is zero build cost to projects that do not actually have any need for tracing, and like I said it seems to have been the original intention to do so as evidenced by the existence of the default = ["tracing"] that just wasn't wired in.

There is plenty of other code here using #[cfg(feature = ...)], I'd think it'd be in-keeping to do it that way

[ShoyuVanilla]

Personally, I'm not opposing to placing tracing behind the feature flag and it sounds reasonable as we already have that broken feature.

But anyway it makes things a bit complicated anyway. For now, yeah, it's only extra ten lines of code. However we'll have to keep in with that every time we add new tracing span/instrument/events. That's really easy to miss and will break the release. So we might need extra CI step and such things are complexities

[ChayimFriedman2]

Yeah. Without a CI job (which we aren't going to add just for this) it's pretty much guaranteed to break again in the future, and anyway it's a cognitive burden.

The fact that the feature exists does probably mean this was the intended design, but that doesn't mean that's what we want.

However using no-default-features seems fine to me; it is low-effort, and shouldn't cause a maintenance burden.

[lmmx]

OK! I will put in another PR the alternative way, thanks all for the feedback 🙂

[Veykril]

Fyi you'll need to remove the entire .workspace part then. Workspace table inherits can only be changed additively, so just adding the no default features key to that will not change anything as its not additive in that regard

[lmmx]

I think I've got it, you can change the workspace Cargo.toml dep to

-tracing = "0.1.41"
+tracing = { version = "0.1.41", default-features = false, features = ["std"] }

and the {project-model,hir-ty}/Cargo.toml crate deps to activate it since it is used:

-tracing.workspace = true
+tracing = { workspace = true, features = ["attributes"] }

and since the parser crate's tracing dep is not in fact optional, don't say it is in parser/Cargo.toml:

-tracing = { workspace = true, optional = true }
+tracing.workspace = true

and as such must remove the (broken) feature from default feature group:

 [features]
-default = ["tracing"]
+default = []

I have prepared a PR with the workspace dependency changed to just have the std feature, and made the tracing non-optional in the parser crate (since it was broken and wouldn't build anyway).

So to recap this makes r-a more correct on 2 fronts:

• No longer has an unbuildable parser crate when --no-default-features is specified
• No longer has an unused tracing/attributes feature pulling in deps for the parser and syntax crates

The full list of 20 crates that used tracing (via rg tracing -l $(fd -e toml)) is:

rg tracing $(fd -e toml) -l | sort | rg -oP '(?<=crates/)[^/]+' | bat -n

   1 base-db
   2 cfg
   3 hir
   4 hir-def
   5 hir-expand
   6 hir-ty
   7 ide-assists
   8 ide
   9 ide-completion
  10 ide-db
  11 ide-diagnostics
  12 load-cargo
  13 parser
  14 proc-macro-api
  15 project-model
  16 rust-analyzer
  17 stdx
  18 syntax
  19 vfs
  20 vfs-notify

Of these only 6 contain tracing instrumentation requiring the tracing/attributes feature

rg 'instrument\(' $(rg tracing $(fd -e toml) -l | cut -d/ -f-2) -l | cut -d/ -f2 | braces --sort

{hir,hir-def,hir-ty,ide-assists,ide-db,project-model}

The parser and syntax crates no longer pull in the unused deps

• PR: fix: no unused tracing/attributes feature #21114