From e9027818951824df1f2da6d897b5b85ce4e5f744 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicolas=20S=C3=A9verin?= <nicolas.severin@outlook.com>
Date: Wed, 27 Nov 2024 16:11:51 +0000
Subject: [PATCH] Panic on duplicated field names

---
 tracing-attributes/tests/fields.rs | 43 +++++++++++++++++-------------
 tracing-core/src/field.rs          | 35 ++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 19 deletions(-)

diff --git a/tracing-attributes/tests/fields.rs b/tracing-attributes/tests/fields.rs
index 3220d4390..848335df3 100644
--- a/tracing-attributes/tests/fields.rs
+++ b/tracing-attributes/tests/fields.rs
@@ -60,10 +60,14 @@ fn fn_struct_const_field_name() {}
 #[instrument(fields({"foo"} = "bar"))]
 fn fn_string_field_name() {}
 
-const CLASHY_FIELD_NAME: &str = "s";
-
-#[instrument(fields({CLASHY_FIELD_NAME} = "foo"))]
-fn fn_clashy_const_field_name(s: &str) { let _ = s; }
+// TODO(#3158): To be consistent with event! and span! macros, the argument's value should be
+//     dropped, but this is only possible to implement when the compiler supports const evaluation
+//     (https://rustc-dev-guide.rust-lang.org/const-eval). For now the duplicated field would
+//     trigger a panic.
+// const CLASHY_FIELD_NAME: &str = "s";
+//
+// #[instrument(fields({CLASHY_FIELD_NAME} = "foo"))]
+// fn fn_clashy_const_field_name(s: &str) { let _ = s; }
 
 #[derive(Debug)]
 struct HasField {
@@ -235,21 +239,22 @@ fn string_field_name() {
     });
 }
 
-#[test]
-fn clashy_const_field_name() {
-    let span = expect::span().with_fields(
-        // TODO(#3158): to be consistent with event! and span! macros, the argument's value should
-        //       be dropped. but this is only possible to implement when the compiler supports const
-        //       evaluation (https://rustc-dev-guide.rust-lang.org/const-eval).
-        expect::field("s")
-            .with_value(&"foo")
-            .and(expect::field("s").with_value(&"hello world"))
-            .only(),
-    );
-    run_test(span, || {
-        fn_clashy_const_field_name("hello world");
-    });
-}
+// TODO(#3158): To be consistent with event! and span! macros, the argument's value should be
+//     dropped, but this is only possible to implement when the compiler supports const evaluation
+//     (https://rustc-dev-guide.rust-lang.org/const-eval). For now the duplicated field would
+//     trigger a panic.
+// #[test]
+// fn clashy_const_field_name() {
+//     let span = expect::span().with_fields(
+//         expect::field("s")
+//             .with_value(&"foo")
+//             .and(expect::field("s").with_value(&"hello world"))
+//             .only(),
+//     );
+//     run_test(span, || {
+//         fn_clashy_const_field_name("hello world");
+//     });
+// }
 
 fn run_test<F: FnOnce() -> T, T>(span: NewSpan, fun: F) {
     let (collector, handle) = collector::mock()
diff --git a/tracing-core/src/field.rs b/tracing-core/src/field.rs
index 6b4c95090..43eef46c7 100644
--- a/tracing-core/src/field.rs
+++ b/tracing-core/src/field.rs
@@ -730,11 +730,46 @@ impl Clone for Field {
     }
 }
 
+// Compares two strings for equality in a constant context.
+const fn str_eq(left: &str, right: &str) -> bool {
+    if left.len() != right.len() {
+        return false;
+    }
+
+    let left_bytes = left.as_bytes();
+    let right_bytes = right.as_bytes();
+    let mut i = left_bytes.len();
+    while i > 0 {
+        if left_bytes[i - 1] != right_bytes[i - 1] {
+            return false;
+        }
+        i = i - 1;
+    }
+
+    true
+}
+
+// Asserts field names are distinct.
+const fn assert_distinct(names: &[&str]) {
+    let mut i = names.len();
+    while i > 0 {
+        let mut j = i - 1;
+        while j > 0 {
+            if str_eq(names[i - 1], names[j - 1]) {
+                panic!("duplicated field name");
+            }
+            j = j - 1;
+        }
+        i = i - 1;
+    }
+}
+
 // ===== impl FieldSet =====
 
 impl FieldSet {
     /// Constructs a new `FieldSet` with the given array of field names and callsite.
     pub const fn new(names: &'static [&'static str], callsite: callsite::Identifier) -> Self {
+        assert_distinct(names);
         Self { names, callsite }
     }