Skip to content

Allow tidy check in codegen#153166

Draft
reddevilmidzy wants to merge 9 commits intorust-lang:mainfrom
reddevilmidzy:codegen-tidy
Draft

Allow tidy check in codegen#153166
reddevilmidzy wants to merge 9 commits intorust-lang:mainfrom
reddevilmidzy:codegen-tidy

Conversation

@reddevilmidzy
Copy link
Member

@reddevilmidzy reddevilmidzy commented Feb 27, 2026

Fixes: #152280
MCP: rust-lang/compiler-team#963

I've separated the commits for easier review.

r? lcnr

@rustbot rustbot added A-tidy Area: The tidy tool S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 27, 2026
Comment on lines +265 to +266
// Patch files may include tidy markers unrelated to source ordering.
|| path.extension() == Some(OsStr::new("patch"))
Copy link
Member Author

Choose a reason for hiding this comment

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

The .patch was allowed because of this file.

+++ b/coretests/tests/lib.rs
@@ -2,4 +2,3 @@
// tidy-alphabetical-start
-#![cfg_attr(target_has_atomic = "128", feature(integer_atomics))]
#![feature(array_ptr_get)]

with open(output_file, "w", encoding="utf8") as out:
out.write("""// File generated by `rustc_codegen_gcc/tools/generate_intrinsics.py`
// DO NOT EDIT IT!
// ignore-tidy-filelength
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 added // ignore-tidy-filelength here and also manually added it to compiler/rustc_codegen_gcc/src/intrinsic/archs.rs.

@@ -65,6 +65,7 @@ pub fn check(root_path: &Path, stdlib: bool, tidy_ctx: TidyCtx) {
|| path.ends_with("library/alloc/src/collections/vec_deque/tests.rs")
|| path.ends_with("library/alloc/src/raw_vec/tests.rs")
|| path.ends_with("library/alloc/src/wtf8/tests.rs")
|| path.ends_with("compiler/rustc_codegen_gcc/build_system/src/utils.rs")
Copy link
Member Author

Choose a reason for hiding this comment

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

#[test]
fn test_split_args() {
// Missing `"` at the end.
assert!(split_args("\"tada").is_err());
// Missing `'` at the end.
assert!(split_args("\'tada").is_err());
assert_eq!(
split_args("a \"b\" c"),
Ok(vec!["a".to_string(), "\"b\"".to_string(), "c".to_string()])
);
// Trailing whitespace characters.
assert_eq!(
split_args(" a \"b\" c "),
Ok(vec!["a".to_string(), "\"b\"".to_string(), "c".to_string()])
);
}

Because of this test, should I move the test or allow this?
It felt like a strong unit test, and I didn't see a good place for it, so I allowed it for now.

@reddevilmidzy reddevilmidzy marked this pull request as ready for review February 27, 2026 06:21
@rustbot
Copy link
Collaborator

rustbot commented Feb 27, 2026

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 27, 2026
@bjorn3
Copy link
Member

bjorn3 commented Feb 27, 2026

Tidy does not run inside the cg_clif and cg_gcc repos, which means whenever we do a sync, there is a fair chance that we did be forced to male changes when tidy complains.

@jieyouxu
Copy link
Member

jieyouxu commented Feb 27, 2026

Tidy does not run inside the cg_clif and cg_gcc repos, which means whenever we do a sync, there is a fair chance that we did be forced to male changes when tidy complains.

I would suggest not running full tidy against the cg_* subtrees yeah.

Also, doesn't rust-lang/compiler-team#963 only say TODO -> FIXME? Not other tidy checks? I do not think we should be enforcing other tidy rules for the cg_* subtrees.

@reddevilmidzy
Copy link
Member Author

Ah, I overinterpreted it — my mistake. Thanks for the clarification. I’ll update it accordingly.

@reddevilmidzy reddevilmidzy marked this pull request as draft February 27, 2026 10:58
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 27, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 2, 2026

☔ The latest upstream changes (presumably #153278) made this pull request unmergeable. Please resolve the merge conflicts.

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

Labels

A-tidy Area: The tidy tool S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tracking issue for MCP 963: Extend the x.py policy for TODO and FIXME to other in-tree projects

5 participants