Panic on duplicated field names

tracing-attributes/src/lib.rs

@@ -111,11 +111,12 @@ mod expand;
 /// to the generated span through the `fields` argument on the
 /// `#[instrument]` macro. Arbitrary expressions are accepted as value
 /// for each field. The name of the field must be a single valid Rust
-/// identifier, or a constant expression that evaluates to one enclosed in curly
+/// identifier, or a constant expression that evaluates to one, enclosed in curly
 /// braces. Note that nested (dotted) field names are supported.
 ///
-/// Note that defining a field with the same name as a (non-explicitly-skipped)
-/// argument will implicitly skip the argument.
+/// Note that defining a field with the same name as a (non-skipped)
+/// argument will implicitly skip the argument, or even panic if using
+/// constants as field names.
 ///
 /// # Examples
 /// Instrumenting a function:

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()

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 }
     }