Merge branch 'main' into konsti/fix-cc-lower-bound

Author: GuillaumeLagrange

crates/codspeed/src/utils.rs

@@ -32,6 +32,37 @@ where
     }
 }
 
+/// Builds a benchmark URI by joining the non-empty segments with `::`.
+///
+/// Segments can be empty when `criterion_group!`/`criterion_main!` are bypassed (custom main):
+/// `current_file` and `macro_group` are not set in that case and must not produce empty
+/// `::` parts (e.g. `::::my_group::my_bench`).
+pub fn build_uri(segments: &[&str]) -> String {
+    segments
+        .iter()
+        .filter(|s| !s.is_empty())
+        .copied()
+        .collect::<Vec<_>>()
+        .join("::")
+}
+
+/// Resolves the caller's source file path for URI generation, anchored to the
+/// workspace root when running under the CodSpeed runner.
+///
+/// `#[track_caller]` so the location resolves to the original benchmark call site
+/// rather than this helper.
+#[track_caller]
+pub fn caller_file_path() -> String {
+    let caller = std::panic::Location::caller();
+    match std::env::var("CODSPEED_CARGO_WORKSPACE_ROOT") {
+        Ok(workspace_root) => PathBuf::from(workspace_root)
+            .join(caller.file())
+            .to_string_lossy()
+            .into_owned(),
+        Err(_) => caller.file().to_string(),
+    }
+}
+
 /// Fixes spaces around `::` created by stringify!($function).
 pub fn get_formated_function_path(function_path: impl Into<String>) -> String {
     let function_path = function_path.into();
@@ -111,6 +142,28 @@ mod tests {
         assert_eq!(relative_path, symlink.canonicalize().unwrap());
     }
 
+    /// COD-2324: empty segments (no file/macro group when `criterion_group!` is bypassed)
+    /// must not produce URIs like `::::my_group::my_bench`.
+    #[test]
+    fn test_build_uri_skips_empty_segments() {
+        assert_eq!(
+            build_uri(&["", "", "my_group::my_bench"]),
+            "my_group::my_bench"
+        );
+        assert_eq!(
+            build_uri(&["benches/custom_main.rs", "", "my_group::my_bench"]),
+            "benches/custom_main.rs::my_group::my_bench"
+        );
+        assert_eq!(
+            build_uri(&[
+                "benches/bench.rs",
+                "benches::bench_fn",
+                "my_group::my_bench"
+            ]),
+            "benches/bench.rs::benches::bench_fn::my_group::my_bench"
+        );
+    }
+
     #[test]
     fn test_get_formated_function_path() {
         let input = "std :: vec :: Vec :: new";

crates/criterion_compat/Cargo.toml

@@ -68,3 +68,7 @@ harness = false
 [[bench]]
 name = "test_benches"
 harness = false
+
+[[bench]]
+name = "custom_main"
+harness = false

crates/criterion_compat/benches/custom_main.rs

@@ -0,0 +1,42 @@
+/// Test bench for custom main patterns (COD-2324).
+/// Verifies URI formatting when users bypass criterion_group!/criterion_main!.
+use codspeed_criterion_compat::{criterion_group, BenchmarkId, Criterion};
+
+fn bench_with_group(c: &mut Criterion) {
+    let mut group = c.benchmark_group("my_group");
+    group.bench_function("my_bench", |b| b.iter(|| 2 + 2));
+    group.bench_function(BenchmarkId::new("parameterized", 42), |b| b.iter(|| 2 + 2));
+    group.finish();
+}
+
+criterion_group!(benches, bench_with_group);
+
+#[cfg(codspeed)]
+fn main() {
+    // Pattern A: Using new_instrumented() but calling bench functions directly (not through criterion_group!)
+    // Since criterion_group! is bypassed, there is no macro context (group/function name),
+    // so the URI only contains the file, benchmark group, and benchmark name.
+    //
+    // Expected URIs:
+    // - crates/criterion_compat/benches/custom_main.rs::my_group::my_bench
+    // - crates/criterion_compat/benches/custom_main.rs::my_group::parameterized[42]
+    let mut criterion = Criterion::new_instrumented();
+    bench_with_group(&mut criterion);
+
+    // Pattern B: Calling through criterion_group!-generated function (should work correctly)
+    // The macro captures the criterion_group! name (`benches`) and the bench function name
+    // (`bench_with_group`), so both are part of the URI.
+    //
+    // Expected URIs:
+    // - crates/criterion_compat/benches/custom_main.rs::benches::bench_with_group::my_group::my_bench
+    // - crates/criterion_compat/benches/custom_main.rs::benches::bench_with_group::my_group::parameterized[42]
+    let mut criterion2 = Criterion::new_instrumented();
+    benches(&mut criterion2);
+}
+
+#[cfg(not(codspeed))]
+fn main() {
+    // Without codspeed, just run through upstream criterion directly
+    let mut criterion = Criterion::default().configure_from_args();
+    bench_with_group(&mut criterion);
+}

crates/criterion_compat/criterion_fork/src/analysis/mod.rs

@@ -279,12 +279,11 @@ mod codspeed {
         }
 
         let git_relative_file_path = codspeed::utils::get_git_relative_path(&c.current_file);
-        let uri = format!(
-            "{}::{}::{}",
-            git_relative_file_path.to_string_lossy(),
+        let uri = codspeed::utils::build_uri(&[
+            &git_relative_file_path.to_string_lossy(),
             &c.macro_group,
-            bench_name
-        );
+            &bench_name,
+        ]);
 
         (uri, bench_name)
     }

crates/criterion_compat/criterion_fork/src/lib.rs

@@ -398,6 +398,18 @@ mod codspeed {
         pub fn set_macro_group(&mut self, macro_group: impl Into<String>) {
             self.macro_group = macro_group.into();
         }
+
+        /// Ensures `current_file` is set, for CodSpeed URI generation.
+        ///
+        /// When set explicitly via `set_current_file`, it is kept as-is; otherwise it is
+        /// derived from the caller location.
+        #[track_caller]
+        pub(crate) fn ensure_current_file(&mut self) {
+            if !self.current_file.is_empty() {
+                return;
+            }
+            self.current_file = codspeed::utils::caller_file_path();
+        }
     }
 }
 
@@ -1201,7 +1213,9 @@ https://bheisler.github.io/criterion.rs/book/faq.html
     /// ```
     /// # Panics:
     /// Panics if the group name is empty
+    #[track_caller]
     pub fn benchmark_group<S: Into<String>>(&mut self, group_name: S) -> BenchmarkGroup<'_, M> {
+        self.ensure_current_file();
         let group_name = group_name.into();
         assert!(!group_name.is_empty(), "Group name must not be empty.");
 
@@ -1238,6 +1252,7 @@ where
     /// criterion_group!(benches, bench);
     /// criterion_main!(benches);
     /// ```
+    #[track_caller]
     pub fn bench_function<F>(&mut self, id: &str, f: F) -> &mut Criterion<M>
     where
         F: FnMut(&mut Bencher<'_, M>),
@@ -1270,6 +1285,7 @@ where
     /// criterion_group!(benches, bench);
     /// criterion_main!(benches);
     /// ```
+    #[track_caller]
     pub fn bench_with_input<F, I>(&mut self, id: BenchmarkId, input: &I, f: F) -> &mut Criterion<M>
     where
         F: FnMut(&mut Bencher<'_, M>, &I),

crates/criterion_compat/src/compat/criterion.rs

@@ -92,6 +92,7 @@ impl<M: Measurement> Criterion<M> {
         self.macro_group = macro_group.into();
     }
 
+    #[track_caller]
     pub fn bench_function<F>(&mut self, id: &str, f: F) -> &mut Criterion<M>
     where
         F: FnMut(&mut Bencher),
@@ -101,6 +102,7 @@ impl<M: Measurement> Criterion<M> {
         self
     }
 
+    #[track_caller]
     pub fn bench_with_input<F, I>(&mut self, id: BenchmarkId, input: &I, f: F) -> &mut Criterion<M>
     where
         F: FnMut(&mut Bencher, &I),
@@ -118,9 +120,23 @@ impl<M: Measurement> Criterion<M> {
         self
     }
 
+    #[track_caller]
     pub fn benchmark_group<S: Into<String>>(&mut self, group_name: S) -> BenchmarkGroup<M> {
+        self.ensure_current_file();
         BenchmarkGroup::<M>::new(self, group_name.into())
     }
+
+    /// Ensures `current_file` is set, for CodSpeed URI generation.
+    ///
+    /// When set explicitly via `set_current_file`, it is kept as-is; otherwise it is
+    /// derived from the caller location.
+    #[track_caller]
+    fn ensure_current_file(&mut self) {
+        if !self.current_file.is_empty() {
+            return;
+        }
+        self.current_file = codspeed::utils::caller_file_path();
+    }
 }
 
 // Dummy methods

crates/criterion_compat/src/compat/group.rs

@@ -1,7 +1,10 @@
 use std::marker::PhantomData;
 use std::{cell::RefCell, rc::Rc, time::Duration};
 
-use codspeed::{codspeed::CodSpeed, utils::get_git_relative_path};
+use codspeed::{
+    codspeed::CodSpeed,
+    utils::{build_uri, get_git_relative_path},
+};
 use criterion::measurement::WallTime;
 use criterion::{measurement::Measurement, PlotConfiguration, SamplingMode, Throughput};
 
@@ -63,12 +66,11 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> {
         I: ?Sized,
     {
         let git_relative_file_path = get_git_relative_path(&self.current_file);
-        let mut uri = format!(
-            "{}::{}::{}",
-            git_relative_file_path.to_string_lossy(),
-            self.macro_group,
-            self.group_name,
-        );
+        let mut uri = build_uri(&[
+            &git_relative_file_path.to_string_lossy(),
+            &self.macro_group,
+            &self.group_name,
+        ]);
         if let Some(function_name) = id.function_name {
             uri = format!("{uri}::{function_name}");
         }