Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverse dependency between clippy_utils and clippy_config #13691

Merged
merged 3 commits into from
Nov 15, 2024

Conversation

flip1995
Copy link
Member

@flip1995 flip1995 commented Nov 15, 2024

In preparation of #13556, I want to remove the dependency on clippy_config, as I don't think that we want to publish that for outside consumers. To do this the 2 dependecies on clippy_config had to be removed:

  1. The MSRV implementation was in clippy_config, but was required in qualify_min_const. I think exposing the MSRV infrastructure and the MSRVs we defined might also be helpful for clippy_utils users. I don't see why it should not be able to live in clippy_utils from a technical point of few.
  2. The create_disallowed_map function that took in a clippy_utils::types::DisallowedPath is moved to the DisallowedPath implementation. This also fits there and is only useful for Clippy and not in clippy_utils for external consumers.

clippy_config now depends in clippy_utils, so the dependecy just got reversed. But having the clippy_utils crate as the base of the dependency tree in Clippy makes sense.

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Nov 15, 2024

r? @Centri3

rustbot has assigned @Centri3.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 15, 2024
@flip1995
Copy link
Member Author

This PR is best reviewed by just reviewing the first 2 commits. The last commit is just an automated renaming using sed and cargo dev fmt.

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! ❤️

Comment on lines 16 to 23
extern crate rustc_ast;
extern crate rustc_attr;
#[allow(unused_extern_crates)]
extern crate rustc_driver;
extern crate rustc_errors;
extern crate rustc_session;
extern crate rustc_span;
extern crate smallvec;
Copy link
Member

Choose a reason for hiding this comment

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

I really like the removal of these dependencies (although they seemed to not be used, so I'm not sure if they ended up being linked to in the final executable).

I'm currently looking into the possibility of if being tied to rustc_driver*.so could be slowing us down by a considerable factor.

@blyxyas blyxyas added this pull request to the merge queue Nov 15, 2024
Merged via the queue into rust-lang:master with commit 627363e Nov 15, 2024
9 checks passed
@flip1995 flip1995 deleted the clippy-utils-release-prep branch November 15, 2024 20:20
github-merge-queue bot pushed a commit that referenced this pull request Nov 17, 2024
Follow up to #13691

Adds metadata to the `clippy_utils/Cargo.toml`, which is mostly copied
from the root `Cargo.toml`.
Adds a `README.md` file listing the nightly version `clippy_utils` can
be used with, mentions that there are no stability guarantees and the
license.

The next PR will add automation to update the nightly toolchains in
those files and the versions in the `Cargo.toml`s.

cc #13556

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants