Skip to content

refactor: separate "global" mode from CompileMode#15601

Merged
ehuss merged 2 commits intorust-lang:masterfrom
weihanglo:compilemode
May 29, 2025
Merged

refactor: separate "global" mode from CompileMode#15601
ehuss merged 2 commits intorust-lang:masterfrom
weihanglo:compilemode

Conversation

@weihanglo
Copy link
Member

@weihanglo weihanglo commented May 27, 2025

What does this PR try to resolve?

This separates the concern of two different "mode".

  • UserIntent: focus on the overall goal of the build
  • CompileMode: the actual compile operation for each unit (I'd like to rename it to something else in the future, such as CompileAction)

This is a preparation of adding -Zno-link/-Zlink-only support,
which we'll have CompileMode::Link but that doesn't make sense to
show up in UserIntent.

How should we test and review this PR?

It should have no functional change.

Additional information

@rustbot
Copy link
Collaborator

rustbot commented May 27, 2025

r? @ehuss

rustbot has assigned @ehuss.
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 A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) A-cfg-expr Area: Platform cfg expressions A-cli Area: Command-line interface, option parsing, etc. A-layout Area: target output directory layout, naming, and organization A-lto Area: link-time optimization A-timings Area: timings Command-bench Command-build Command-check Command-doc Command-fetch Command-fix Command-install Command-package Command-run Command-rustc Command-rustdoc Command-test S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 27, 2025
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

pub enum UserIntent {
/// Build benchmark binaries, e.g., `cargo bench`
Bench,
/// Build binaries and libraray, e.g., `cargo run`, `cargo install`, `cargo build`.
Copy link
Member

Choose a reason for hiding this comment

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

*library

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually should be plural 😬.

Copy link
Member Author

Choose a reason for hiding this comment

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

///
/// If `deps` is true, then it will also document all dependencies.
/// if `json` is true, the documentation output is in json format.
Doc { deps: bool, json: bool },
Copy link
Member

Choose a reason for hiding this comment

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

Why is the deps field on CompileMode too and not just on UserIntent?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because you need it to determine whether to include deps for documenting, during computing the unit graph. I haven't found a good way to remove that.

if let CompileMode::Doc { deps: true, .. } = unit.mode {
// Document this lib as well.
let doc_unit_dep = new_unit_dep(
state,
unit,
dep_pkg,
dep_lib,
dep_unit_for,
unit.kind.for_target(dep_lib),
unit.mode,
IS_NO_ARTIFACT_DEP,
)?;
ret.push(doc_unit_dep);
}

Copy link
Member

Choose a reason for hiding this comment

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

I figured you would have access to the UserIntent when deciding if a crate should be documented, but I guess not.

Copy link
Member Author

Choose a reason for hiding this comment

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

I missed that. You're right.

Copy link
Member Author

Choose a reason for hiding this comment

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

Went ahead and removed the deps field as well. Thanks for the good catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-build-execution Area: anything dealing with executing the compiler A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) 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