Add TODO checker to cg_gcc

.cspell.json

@@ -22,7 +22,7 @@
         "src/intrinsic/llvm.rs"
     ],
     "ignoreRegExpList": [
-        "/(FIXME|NOTE|TODO)\\([^)]+\\)/",
+        "/(FIXME|NOTE)\\([^)]+\\)/",
         "__builtin_\\w*"
     ]
 }

.github/workflows/ci.yml

@@ -88,6 +88,9 @@ jobs:
     - name: Check formatting
       run: ./y.sh fmt --check
 
+    - name: Check todo
+      run: ./y.sh check-todo
+
     - name: clippy
       run: |
         cargo clippy --all-targets -- -D warnings

.github/workflows/failures.yml

@@ -1,4 +1,4 @@
-# FIXME: refactor to avoid duplication with the ci.yml file.
+# FIXME: This workflow duplicates ci.yml.
 name: Failures
 
 on:
@@ -94,15 +94,15 @@ jobs:
       run: ./y.sh prepare
 
     - name: Run tests
-      # FIXME: re-enable those tests for libgccjit 12.
+      # FIXME: libgccjit 12 still fails these tests.
       if: matrix.libgccjit_version.gcc != 'libgccjit12.so'
       id: tests
       run: |
         ${{ matrix.libgccjit_version.env_extra }} ./y.sh test --release --clean --build-sysroot --test-failing-rustc ${{ matrix.libgccjit_version.extra }} 2>&1 | tee output_log
         rg --text "test result" output_log >> $GITHUB_STEP_SUMMARY
 
     - name: Run failing ui pattern tests for ICE
-      # FIXME: re-enable those tests for libgccjit 12.
+      # FIXME: libgccjit 12 still fails these tests.
       if: matrix.libgccjit_version.gcc != 'libgccjit12.so'
       id: ui-tests
       run: |

.github/workflows/m68k.yml

@@ -1,4 +1,4 @@
-# FIXME: check if qemu-user-static-binfmt is needed (perhaps to run some tests since it probably calls exec).
+# FIXME: It is still unclear whether qemu-user-static-binfmt is required for tests that call exec.
 
 name: m68k CI
 
@@ -24,7 +24,7 @@ jobs:
       matrix:
         commands: [
           "--std-tests",
-          # FIXME(antoyo): fix those on m68k.
+          # FIXME(antoyo): These tests are still broken on m68k.
           #"--test-libcore",
           #"--extended-rand-tests",
           #"--extended-regex-example-tests",

.github/workflows/stdarch.yml

@@ -38,7 +38,7 @@ jobs:
     - name: Install packages
       run: sudo apt-get install ninja-build ripgrep
 
-    # FIXME: remove when we have binutils version 2.43 in the repo.
+    # FIXME: This binutils workaround is required until the repo ships binutils 2.43.
     - name: Install more recent binutils
       run: |
         echo "deb http://archive.ubuntu.com/ubuntu plucky main universe" | sudo tee /etc/apt/sources.list.d/plucky-copies.list
@@ -96,7 +96,7 @@ jobs:
       if: ${{ matrix.cargo_runner }}
       run: |
         # FIXME: these tests fail when the sysroot is compiled with LTO because of a missing symbol in proc-macro.
-        # FIXME: remove --skip test_tile_ when it's implemented.
+        # FIXME: test_tile_ must stay skipped until the tile builtins are implemented.
         STDARCH_TEST_SKIP_FUNCTION="xsave,xsaveopt,xsave64,xsaveopt64" STDARCH_TEST_EVERYTHING=1 CHANNEL=release CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="${{ matrix.cargo_runner }}" TARGET=x86_64-unknown-linux-gnu CG_RUSTFLAGS="-Ainternal_features" ./y.sh cargo test --manifest-path build/build_sysroot/sysroot_src/library/stdarch/Cargo.toml -- --skip rtm --skip tbm --skip sse4a --skip test_tile_
 
   # Summary job for the merge queue.

build_system/src/main.rs

@@ -12,6 +12,7 @@ mod prepare;
 mod rust_tools;
 mod rustc_info;
 mod test;
+mod todo;
 mod utils;
 const BUILD_DIR: &str = "build";
 
@@ -45,7 +46,8 @@ Commands:
         clone-gcc : Clones the GCC compiler from a specified source.
         fmt       : Runs rustfmt
         fuzz      : Fuzzes `cg_gcc` using rustlantis
-        abi-test   : Runs the abi-cafe test suite on the codegen, checking for ABI compatibility with LLVM"
+        abi-test   : Runs the abi-cafe test suite on the codegen, checking for ABI compatibility with LLVM
+        check-todo : Checks todo in the project"
     );
 }
 
@@ -61,6 +63,7 @@ pub enum Command {
     Fmt,
     Fuzz,
     AbiTest,
+    CheckTodo,
 }
 
 fn main() {
@@ -80,6 +83,7 @@ fn main() {
         Some("info") => Command::Info,
         Some("clone-gcc") => Command::CloneGcc,
         Some("abi-test") => Command::AbiTest,
+        Some("check-todo") => Command::CheckTodo,
         Some("fmt") => Command::Fmt,
         Some("fuzz") => Command::Fuzz,
         Some("--help") => {
@@ -106,6 +110,7 @@ fn main() {
         Command::Fmt => fmt::run(),
         Command::Fuzz => fuzz::run(),
         Command::AbiTest => abi_test::run(),
+        Command::CheckTodo => todo::run(),
     } {
         eprintln!("Command failed to run: {e}");
         process::exit(1);

build_system/src/prepare.rs

@@ -148,7 +148,7 @@ fn prepare_libcore(
     Ok(())
 }
 
-// FIXME: remove when we can ignore warnings in rustdoc tests.
+// FIXME: This workaround is still required because rustdoc tests cannot ignore warnings.
 fn prepare_rand() -> Result<(), String> {
     // Apply patch for the rand crate.
     let file_path = "patches/crates/0001-Remove-deny-warnings.patch";

build_system/src/test.rs

@@ -286,7 +286,7 @@ fn build_sysroot(env: &Env, args: &TestArg) -> Result<(), String> {
     Ok(())
 }
 
-// FIXME(GuillaumeGomez): when rewriting in Rust, refactor with the code in tests/lang_tests_common.rs if possible.
+// FIXME(GuillaumeGomez): This still duplicates tests/lang_tests_common.rs; keep them in sync until the Rust rewrite.
 fn maybe_run_command_in_vm(
     command: &[&dyn AsRef<OsStr>],
     env: &Env,
@@ -708,16 +708,16 @@ fn test_projects(env: &Env, args: &TestArg) -> Result<(), String> {
         "https://github.com/BurntSushi/memchr",
         "https://github.com/dtolnay/itoa",
         "https://github.com/rust-lang/cfg-if",
-        //"https://github.com/rust-lang-nursery/lazy-static.rs", // FIXME: re-enable when the
+        //"https://github.com/rust-lang-nursery/lazy-static.rs", // FIXME: This stays disabled until the
         //failing test is fixed upstream.
         //"https://github.com/marshallpierce/rust-base64", // FIXME: one test is OOM-killed.
-        // FIXME: ignore the base64 test that is OOM-killed.
+        // FIXME: The OOM-killed base64 test still needs to be ignored.
         //"https://github.com/time-rs/time", // FIXME: one test fails (https://github.com/time-rs/time/issues/719).
         "https://github.com/rust-lang/log",
         "https://github.com/bitflags/bitflags",
         //"https://github.com/serde-rs/serde", // FIXME: one test fails.
-        //"https://github.com/rayon-rs/rayon", // FIXME: very slow, only run on master?
-        //"https://github.com/rust-lang/cargo", // FIXME: very slow, only run on master?
+        //"https://github.com/rayon-rs/rayon", // FIXME: This is too slow for regular CI runs; keep it for master-only runs if re-enabled.
+        //"https://github.com/rust-lang/cargo", // FIXME: This is too slow for regular CI runs; keep it for master-only runs if re-enabled.
     ];
 
     let mut env = env.clone();
@@ -759,7 +759,7 @@ fn test_libcore(env: &Env, args: &TestArg) -> Result<(), String> {
     println!("[TEST] libcore");
     let path = get_sysroot_dir().join("sysroot_src/library/coretests");
     let _ = remove_dir_all(path.join("target"));
-    // FIXME(antoyo): run in release mode when we fix the failures.
+    // FIXME(antoyo): test_libcore still fails in release mode.
     run_cargo_command(&[&"test"], Some(&path), env, args)?;
     Ok(())
 }

build_system/src/todo.rs

@@ -0,0 +1,69 @@
+use std::ffi::OsStr;
+use std::fs;
+use std::path::{Path, PathBuf};
+use std::process::Command;
+
+const EXTENSIONS: &[&str] =
+    &["rs", "py", "js", "sh", "c", "cpp", "h", "md", "css", "ftl", "toml", "yml", "yaml"];
+
+fn has_supported_extension(path: &Path) -> bool {
+    path.extension().is_some_and(|ext| EXTENSIONS.iter().any(|e| ext == OsStr::new(e)))
+}
+
+fn list_tracked_files() -> Result<Vec<PathBuf>, String> {
+    let output = Command::new("git")
+        .args(["ls-files", "-z"])
+        .output()
+        .map_err(|e| format!("Failed to run `git ls-files`: {e}"))?;
+
+    if !output.status.success() {
+        let stderr = String::from_utf8_lossy(&output.stderr);
+        return Err(format!("`git ls-files` failed: {stderr}"));
+    }
+
+    let mut files = Vec::new();
+    for entry in output.stdout.split(|b| *b == 0) {
+        if entry.is_empty() {
+            continue;
+        }
+        let path = std::str::from_utf8(entry).unwrap();
+        files.push(PathBuf::from(path));
+    }
+
+    Ok(files)
+}
+
+pub(crate) fn run() -> Result<(), String> {
+    let files = list_tracked_files()?;
+    let mut error_count = 0;
+    // Avoid embedding the task marker in source so greps only find real occurrences.
+    let todo_marker = "todo".to_ascii_uppercase();
+
+    for file in files {
+        if !has_supported_extension(&file) {
+            continue;
+        }
+
+        let bytes = fs::read(&file).unwrap();
+        let contents = std::str::from_utf8(&bytes).unwrap();
+
+        for (i, line) in contents.split('\n').enumerate() {
+            let trimmed = line.trim();
+            if trimmed.contains(&todo_marker) {
+                eprintln!(
+                    "{}:{}: {} is used for tasks that should be done before merging a PR; if you want to leave a message in the codebase use FIXME",
+                    file.display(),
+                    i + 1,
+                    todo_marker
+                );
+                error_count += 1;
+            }
+        }
+    }
+
+    if error_count == 0 {
+        return Ok(());
+    }
+
+    Err(format!("found {} {}(s)", error_count, todo_marker))
+}

doc/debugging-libgccjit.md

@@ -71,4 +71,4 @@ Maybe by calling the following at the beginning of gdb:
 set substitute-path /usr/src/debug/gcc /path/to/gcc-repo/gcc
 ```
 
-FIXME(antoyo): but that's not what I remember I was doing.
+FIXME(antoyo): These debugging notes are stale; this is not what I remember doing.

doc/subtree.md

@@ -47,6 +47,6 @@ git push
 PATH="$HOME/bin:$PATH" ~/bin/git-subtree push -P compiler/rustc_codegen_gcc/ ../rustc_codegen_gcc/ sync_branch_name
 ```
 
-FIXME: write a script that does the above.
+FIXME: This subtree push flow is still manual.
 
 https://rust-lang.zulipchat.com/#narrow/stream/301329-t-devtools/topic/subtree.20madness/near/258877725

example/mini_core_hello_world.rs

@@ -275,7 +275,7 @@ fn main() {
         }
     }
 
-    // FIXME(antoyo): to make this work, support weak linkage.
+    // FIXME(antoyo): This example still depends on unsupported weak linkage.
     //unsafe { assert_eq!(ABC as usize, 0); }
 
     &mut (|| Some(0 as *const ())) as &mut dyn FnMut() -> Option<*const ()>;

src/abi.rs

@@ -105,7 +105,7 @@ pub struct FnAbiGcc<'gcc> {
 }
 
 pub trait FnAbiGccExt<'gcc, 'tcx> {
-    // FIXME(antoyo): return a function pointer type instead?
+    // FIXME(antoyo): This should return a function pointer type instead of the current representation.
     fn gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> FnAbiGcc<'gcc>;
     fn ptr_to_gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> Type<'gcc>;
     #[cfg(feature = "master")]
@@ -260,7 +260,7 @@ pub fn conv_to_fn_attribute<'gcc>(conv: CanonAbi, arch: &Arch) -> Option<FnAttri
             &Arch::Nvptx64 => FnAttribute::NvptxKernel,
             arch => panic!("Arch {arch} does not support GpuKernel calling convention"),
         },
-        // FIXME(antoyo): check if those AVR attributes are mapped correctly.
+        // FIXME(antoyo): The AVR attribute mapping is still unverified.
         CanonAbi::Interrupt(interrupt_kind) => match interrupt_kind {
             InterruptKind::Avr => FnAttribute::AvrSignal,
             InterruptKind::AvrNonBlocking => FnAttribute::AvrInterrupt,

src/allocator.rs

@@ -98,7 +98,7 @@ fn create_wrapper_function(
     )));
 
     if tcx.sess.must_emit_unwind_tables() {
-        // FIXME(antoyo): emit unwind tables.
+        // FIXME(antoyo): Unwind tables are still not emitted here.
     }
 
     let block = func.new_block("entry");
@@ -138,6 +138,6 @@ fn create_wrapper_function(
         block.end_with_void_return(None);
     }
 
-    // FIXME(@Commeownist): Check if we need to emit some extra debugging info in certain circumstances
+    // FIXME(@Commeownist): Some cases may still be missing extra debugging info here
     // as described in https://github.com/rust-lang/rust/commit/77a96ed5646f7c3ee8897693decc4626fe380643
 }

src/asm.rs

@@ -308,13 +308,13 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
                 }
 
                 InlineAsmOperandRef::SymFn { instance } => {
-                    // FIXME(@Amanieu): Additional mangling is needed on
+                    // FIXME(@Amanieu): Additional mangling is still missing on
                     // some targets to add a leading underscore (Mach-O)
                     // or byte count suffixes (x86 Windows).
                     constants_len += self.tcx.symbol_name(instance).name.len();
                 }
                 InlineAsmOperandRef::SymStatic { def_id } => {
-                    // FIXME(@Amanieu): Additional mangling is needed on
+                    // FIXME(@Amanieu): Additional mangling is still missing on
                     // some targets to add a leading underscore (Mach-O).
                     constants_len +=
                         self.tcx.symbol_name(Instance::mono(self.tcx, def_id)).name.len();
@@ -440,7 +440,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
             match *piece {
                 InlineAsmTemplatePiece::String(ref string) => {
                     for char in string.chars() {
-                        // FIXME(antoyo): might also need to escape | if rustc doesn't do it.
+                        // FIXME(antoyo): Escaping `|` may still be missing if rustc does not already do it.
                         let escaped_char = match char {
                             '%' => "%%",
                             '{' => "%{",
@@ -496,15 +496,15 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
                         }
 
                         InlineAsmOperandRef::SymFn { instance } => {
-                            // FIXME(@Amanieu): Additional mangling is needed on
+                            // FIXME(@Amanieu): Additional mangling is still missing on
                             // some targets to add a leading underscore (Mach-O)
                             // or byte count suffixes (x86 Windows).
                             let name = self.tcx.symbol_name(instance).name;
                             template_str.push_str(name);
                         }
 
                         InlineAsmOperandRef::SymStatic { def_id } => {
-                            // FIXME(@Amanieu): Additional mangling is needed on
+                            // FIXME(@Amanieu): Additional mangling is still missing on
                             // some targets to add a leading underscore (Mach-O).
                             let instance = Instance::mono(self.tcx, def_id);
                             let name = self.tcx.symbol_name(instance).name;
@@ -564,7 +564,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
                 InlineAsmArch::PowerPC | InlineAsmArch::PowerPC64 => {
                     // "cc" is cr0 on powerpc.
                 }
-                // FIXME(@Commeownist): I'm not 100% sure this one clobber is sufficient
+                // FIXME(@Commeownist): This clobber may still be insufficient
                 // on all architectures. For instance, what about FP stack?
                 _ => {
                     extended_asm.add_clobber("cc");
@@ -578,7 +578,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
             extended_asm.set_volatile_flag(true);
         }
         if !options.contains(InlineAsmOptions::NOSTACK) {
-            // FIXME(@Commeownist): figure out how to align stack
+            // FIXME(@Commeownist): The stack is still left unaligned here
         }
         if dest.is_none() && options.contains(InlineAsmOptions::NORETURN) {
             let builtin_unreachable = self.context.get_builtin_function("__builtin_unreachable");
@@ -647,7 +647,7 @@ fn explicit_reg_to_gcc(reg: InlineAsmReg) -> &'static str {
     // For explicit registers, we have to create a register variable: https://stackoverflow.com/a/31774784/389119
     match reg {
         InlineAsmReg::X86(reg) => {
-            // FIXME(antoyo): add support for vector register.
+            // FIXME(antoyo): Vector registers are still unsupported.
             match reg.reg_class() {
                 X86InlineAsmRegClass::reg_byte => {
                     // GCC does not support the `b` suffix, so we just strip it
@@ -908,16 +908,16 @@ impl<'gcc, 'tcx> AsmCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
                         GlobalAsmOperandRef::SymFn { instance } => {
                             let function = get_fn(self, instance);
                             self.add_used_function(function);
-                            // FIXME(@Amanieu): Additional mangling is needed on
+                            // FIXME(@Amanieu): Additional mangling is still missing on
                             // some targets to add a leading underscore (Mach-O)
                             // or byte count suffixes (x86 Windows).
                             let name = self.tcx.symbol_name(instance).name;
                             template_str.push_str(name);
                         }
 
                         GlobalAsmOperandRef::SymStatic { def_id } => {
-                            // FIXME(antoyo): set the global variable as used.
-                            // FIXME(@Amanieu): Additional mangling is needed on
+                            // FIXME(antoyo): Global variables referenced from global_asm are still not marked as used.
+                            // FIXME(@Amanieu): Additional mangling is still missing on
                             // some targets to add a leading underscore (Mach-O).
                             let instance = Instance::mono(self.tcx, def_id);
                             let name = self.tcx.symbol_name(instance).name;
@@ -944,7 +944,7 @@ impl<'gcc, 'tcx> AsmCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
     }
 
     fn mangled_name(&self, instance: Instance<'tcx>) -> String {
-        // FIXME(@Amanieu): Additional mangling is needed on
+        // FIXME(@Amanieu): Additional mangling is still missing on
         // some targets to add a leading underscore (Mach-O)
         // or byte count suffixes (x86 Windows).
         self.tcx.symbol_name(instance).name.to_string()