Skip to content

Rust: remove pre-1.94 format macro fallback - #22559

Draft
paldepind wants to merge 1 commit into
mainfrom
paldepind-remove-rust-toolchain-workarounds
Draft

paldepind wants to merge 1 commit into
mainfrom
paldepind-remove-rust-toolchain-workarounds

Conversation

@paldepind

@paldepind paldepind commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Context

  1. When we began the update to rust-analyzer 0.0.347 we noted an incompatibility with pre-1.94 Rust toolchain versions.
  2. The commit 7faf81f put a workaround in place for this incompatibility: we we manually expand certain macros.
  3. With Rust: Update rust-analyzer to 0.0.347 and pin toolchain #22493 we moved in a different direction, where we always do extraction with a 1.97.0 Rust toolchain.
  4. The < 1.94 is now dead code.

This PR

The PR essentially just reverts 7faf81f, but it's not an exact revert, since some later commits tweaked things a bit.

Spelled out the PR:

  • Deletes the synthetic reconstruction of format-family macro expansions
  • Remove the dedicated pre-1.94 QL fixture and toolchain installation

Copilot AI balanced review requested due to automatic review settings September 14, 2026 11:22
@paldepind
paldepind requested review from a team as code owners September 14, 2026 11:22
@github-actions github-actions Bot added the Rust Pull requests that update Rust code label Sep 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The cleanup is internally consistent, preserves required environment handling, and includes appropriate validation.

Review tier: Balanced
Findings: None

What changed in this PR

Removes obsolete Rust toolchain overrides now that toolchain pinning is managed independently.

Changes:

  • Removes fixed-toolchain selection, logging, and preinstallation.
  • Uses standard cargo while preserving configured environment variables.
  • Removes the direct ra_ap_toolchain dependency and regenerates Bazel metadata.
File Description
rust/​ql/​test/​setup.sh Stops preinstalling Rust 1.97.
rust/​ql/​integration-tests/​conftest.py Removes integration-test toolchain installation.
rust/​extractor/​src/​toolchain.rs Deletes fixed-toolchain logic.
rust/​extractor/​src/​qltest.rs Runs standard cargo with configured environment.
rust/​extractor/​src/​main.rs Removes toolchain logging and module registration.
rust/​extractor/​src/​config.rs Stops overriding RUSTUP_TOOLCHAIN.
rust/​extractor/​Cargo.toml Removes the direct toolchain dependency.
MODULE.bazel Removes the obsolete direct Bazel repository import.
misc/​bazel/​3rdparty/​tree_sitter_extractors_deps/​ra_ap_toolchain/​BUILD.bazel Deletes obsolete generated alias.
misc/​bazel/​3rdparty/​tree_sitter_extractors_deps/​ra_ap_toolchain-0.0.347/​BUILD.bazel Deletes versioned generated alias.
misc/​bazel/​3rdparty/​tree_sitter_extractors_deps/​crates.bzl Regenerates direct dependency metadata.
misc/​bazel/​3rdparty/​tree_sitter_extractors_deps/​BUILD.bazel Removes top-level toolchain aliases.
Cargo.lock Removes the extractor’s direct dependency entry.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@paldepind
paldepind marked this pull request as draft September 14, 2026 12:16
@paldepind paldepind changed the title Rust: remove fixed toolchain workarounds Rust: remove pre-1.94 format macro fallback Sep 14, 2026
@paldepind
paldepind marked this pull request as ready for review September 14, 2026 12:46
@paldepind
paldepind marked this pull request as draft September 14, 2026 12:57
Drop the synthetic format-family macro expansion path and its dedicated
legacy-toolchain tests now that extraction uses a pinned compatible Rust
toolchain.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@paldepind
paldepind force-pushed the paldepind-remove-rust-toolchain-workarounds branch from c51a5eb to 2241a07 Compare September 14, 2026 13:34
@paldepind
paldepind marked this pull request as ready for review September 14, 2026 13:55

@hvitved hvitved left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DCA run was lacking -X use-database-cache=false -X change-ql-submodule-in-semmle-code=true, so I have started a new run. Otherwise LGTM.

@paldepind
paldepind marked this pull request as draft September 16, 2026 08:17
@paldepind
paldepind marked this pull request as ready for review September 16, 2026 08:17
@paldepind

Copy link
Copy Markdown
Contributor Author

Thanks for starting DCA correctly. There are some extraction errors. I'll investigate.

@paldepind

Copy link
Copy Markdown
Contributor Author

Extraction errors are for DCA projects on an old nightly toolchain. For these projects the format-macro fallback is in fact not dead code.format-macro fallback is not dead code.

I'm putting back in draft to re-consider. We could go ahead as-is, but it might be better just to implement #22493 (comment) to avoid noise in QA.

@paldepind
paldepind marked this pull request as draft September 16, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants