Support #[cxx_name = "operator+="], etc. (i.e. not just C identifiers).

This came up in https://crrev.com/c/7253816/comment/3f283bcf_9ab0850a/
and in #109 (comment)

Author: anforowicz

BUCK

@@ -47,6 +47,7 @@ rust_binary(
         "//third-party:proc-macro2",
         "//third-party:quote",
         "//third-party:syn",
+        "//third-party:unicode-ident",
     ],
 )
 
@@ -76,6 +77,7 @@ rust_library(
         "//third-party:quote",
         "//third-party:rustversion",
         "//third-party:syn",
+        "//third-party:unicode-ident",
     ],
 )
 
@@ -101,6 +103,7 @@ rust_library(
         "//third-party:quote",
         "//third-party:scratch",
         "//third-party:syn",
+        "//third-party:unicode-ident",
     ],
 )
 
@@ -125,5 +128,6 @@ rust_library(
         "//third-party:proc-macro2",
         "//third-party:quote",
         "//third-party:syn",
+        "//third-party:unicode-ident",
     ],
 )

BUILD.bazel

@@ -39,6 +39,7 @@ rust_binary(
         "@crates.io//:proc-macro2",
         "@crates.io//:quote",
         "@crates.io//:syn",
+        "@crates.io//:unicode-ident",
     ],
 )
 
@@ -70,6 +71,7 @@ rust_proc_macro(
         "@crates.io//:proc-macro2",
         "@crates.io//:quote",
         "@crates.io//:syn",
+        "@crates.io//:unicode-ident",
     ],
 )
 
@@ -87,6 +89,7 @@ rust_library(
         "@crates.io//:quote",
         "@crates.io//:scratch",
         "@crates.io//:syn",
+        "@crates.io//:unicode-ident",
     ],
 )
 
@@ -104,5 +107,6 @@ rust_library(
         "@crates.io//:proc-macro2",
         "@crates.io//:quote",
         "@crates.io//:syn",
+        "@crates.io//:unicode-ident",
     ],
 )

gen/build/Cargo.toml

@@ -24,6 +24,7 @@ proc-macro2 = { version = "1.0.74", default-features = false, features = ["span-
 quote = { version = "1.0.35", default-features = false }
 scratch = "1.0.5"
 syn = { version = "2.0.46", default-features = false, features = ["clone-impls", "full", "parsing", "printing"] }
+unicode-ident = "1.0.22"
 
 [dev-dependencies]
 cxx = { version = "1.0", path = "../.." }

gen/cmd/Cargo.toml

@@ -23,6 +23,7 @@ indexmap = "2.9.0"
 proc-macro2 = { version = "1.0.74", default-features = false, features = ["span-locations"] }
 quote = { version = "1.0.35", default-features = false }
 syn = { version = "2.0.46", default-features = false, features = ["clone-impls", "full", "parsing", "printing"] }
+unicode-ident = "1.0.22"
 
 [package.metadata.docs.rs]
 targets = ["x86_64-unknown-linux-gnu"]

gen/lib/Cargo.toml

@@ -18,6 +18,7 @@ indexmap = "2.9.0"
 proc-macro2 = { version = "1.0.74", default-features = false, features = ["span-locations"] }
 quote = { version = "1.0.35", default-features = false }
 syn = { version = "2.0.46", default-features = false, features = ["clone-impls", "full", "parsing", "printing"] }
+unicode-ident = "1.0.22"
 
 [package.metadata.docs.rs]
 targets = ["x86_64-unknown-linux-gnu"]

macro/Cargo.toml

@@ -20,6 +20,7 @@ indexmap = "2.9.0"
 proc-macro2 = "1.0.74"
 quote = "1.0.35"
 syn = { version = "2.0.46", features = ["full"] }
+unicode-ident = "1.0.22"
 
 [dev-dependencies]
 cxx = { version = "1.0", path = ".." }

syntax/check.rs

@@ -1,5 +1,6 @@
 use crate::syntax::atom::Atom::{self, *};
 use crate::syntax::message::Message;
+use crate::syntax::names::ForeignName;
 use crate::syntax::report::Errors;
 use crate::syntax::visit::{self, Visit};
 use crate::syntax::{
@@ -12,7 +13,9 @@ use quote::{quote, ToTokens};
 use std::collections::HashSet;
 use std::fmt::Display;
 use std::sync::LazyLock;
-use syn::{GenericParam, Generics, Lifetime};
+use syn::ext::IdentExt;
+use syn::parse::Parser;
+use syn::{GenericParam, Generics, Ident, Lifetime};
 
 pub(crate) struct Check<'a> {
     apis: &'a [Api],
@@ -476,7 +479,7 @@ fn check_api_type(cx: &mut Check, ety: &ExternType) {
 }
 
 fn check_api_fn(cx: &mut Check, efn: &ExternFn) {
-    check_name(cx, &efn.name);
+    check_fn_name(cx, &efn.name);
     match efn.lang {
         Lang::Cxx | Lang::CxxUnwind => {
             if !efn.generics.params.is_empty() && !efn.trusted {
@@ -725,6 +728,24 @@ fn check_name(cx: &mut Check, name: &Pair) {
         let msg = format!("C++ reserved keyword can't be used as a C++ identifier: {cxx_name}");
         report_error(msg);
     }
+
+    // Most API names need to have a form of a valid C++ identifier.  API names
+    // that allow other forms (e.g. `operator==` for function names) should
+    // check and allow those forms first (e.g. by `check_fn_name`), before
+    // deciding to call `check_name`.
+    if let Err(e) = Ident::parse_any.parse_str(cxx_name) {
+        let msg = format!("Invalid C++ identifier: {e}");
+        report_error(msg);
+    }
+}
+
+/// Checks `name` of a function or a method.
+fn check_fn_name(cx: &mut Check, name: &Pair) {
+    if ForeignName::is_valid_operator_name(name.cxx.as_str()) {
+        return;
+    }
+
+    check_name(cx, name);
 }
 
 /// Checks `name` of a type (e.g. a struct, enum, or a type alias).

syntax/names.rs

@@ -1,8 +1,10 @@
 use crate::syntax::symbol::Segment;
 use crate::syntax::{Lifetimes, NamedType, Pair, Symbol};
 use proc_macro2::{Ident, Span};
+use std::collections::HashSet;
 use std::fmt::{self, Display};
 use std::iter;
+use std::sync::LazyLock;
 use syn::ext::IdentExt;
 use syn::parse::{Error, Parser, Result};
 use syn::punctuated::Punctuated;
@@ -37,8 +39,13 @@ impl NamedType {
 
 impl ForeignName {
     pub(crate) fn parse(text: &str, span: Span) -> Result<Self> {
-        // TODO: support C++ names containing whitespace (`unsigned int`) or
-        // non-alphanumeric characters (`operator++`).
+        if ForeignName::is_valid_operator_name(text) {
+            return Ok(ForeignName {
+                text: text.to_string(),
+                span,
+            });
+        }
+
         match Ident::parse_any.parse_str(text) {
             Ok(ident) => {
                 let text = ident.to_string();
@@ -55,6 +62,26 @@ impl ForeignName {
     pub(crate) fn span(&self) -> Span {
         self.span
     }
+
+    pub(crate) fn is_valid_operator_name(name: &str) -> bool {
+        #[rustfmt::skip]
+        static CPP_OPERATORS: LazyLock<HashSet<&'static str>> = LazyLock::new(|| {
+            // Based on `llvm/llvm-project/clang/include/clang/Basic/OperatorKinds.def`.
+            // Excluding `?` because it is not overridable.
+            //
+            // TODO: Consider also allowing `operator <type>`
+            // (see https://en.cppreference.com/w/cpp/language/cast_operator.html).
+            [
+                " new", " delete", " new[]", " delete[]", " co_await",
+                "+", "-", "*", "/", "%", "^", "&", "|", "~", "!", "=", "<", ">",
+                "+=", "-=", "*=", "/=", "%=", "^=", "&=", "|=",
+                "<<", ">>", "<<=", ">>=", "==", "!=", "<=", ">=", "<=>",
+                "&&", "||", "++", "--", ",", "->*", "->", "()", "[]",
+            ].into_iter().collect()
+        });
+        name.strip_prefix("operator")
+            .is_some_and(|suffix| CPP_OPERATORS.contains(suffix))
+    }
 }
 
 impl Display for ForeignName {

syntax/symbol.rs

@@ -91,9 +91,34 @@ impl Segment for Pair {
 
 impl Segment for ForeignName {
     fn write(&self, symbol: &mut Symbol) {
-        // TODO: support C++ names containing whitespace (`unsigned int`) or
-        // non-alphanumeric characters (`operator++`).
-        self.to_string().write(symbol);
+        /// Escapes arbitrary C++ name (e.g. `operator==`) into a String
+        /// that is a valid C identifier.  It is important that this is an
+        /// [injective function](https://en.wikipedia.org/wiki/Injective_function)
+        /// (i.e. distinct `name`s need to map to distinct results).
+        fn escape(name: &str) -> String {
+            let mut result = String::with_capacity(name.len());
+            for (index, ch) in name.chars().enumerate() {
+                if ch == '_' {
+                    write!(&mut result, "_u").unwrap();
+                    continue;
+                }
+
+                let should_escape = if index == 0 {
+                    !unicode_ident::is_xid_start(ch)
+                } else {
+                    !unicode_ident::is_xid_continue(ch)
+                };
+                if should_escape {
+                    write!(&mut result, "_{:x}h", ch as u32).unwrap();
+                    continue;
+                }
+
+                write!(&mut result, "{ch}").unwrap();
+            }
+            result
+        }
+
+        escape(self.as_str()).write(symbol);
     }
 }
 
@@ -114,3 +139,33 @@ pub(crate) fn join(segments: &[&dyn Segment]) -> Symbol {
     assert!(!symbol.0.is_empty());
     symbol
 }
+
+#[cfg(test)]
+mod test {
+    use super::join;
+    use crate::syntax::ForeignName;
+    use proc_macro2::Span;
+
+    #[test]
+    fn test_impl_segment_for_foreign_name() {
+        fn t(foreign_name_str: &str, expected_symbol_str: &str) {
+            let foreign_name = ForeignName::parse(foreign_name_str, Span::call_site()).unwrap();
+            let symbol = join(&[&foreign_name]);
+            let actual_symbol_str = symbol.to_string();
+            assert_eq!(
+                actual_symbol_str, expected_symbol_str,
+                "Expecting `{foreign_name_str}` to mangle as `{expected_symbol_str}` \
+                 but got `{actual_symbol_str}` instead.",
+            );
+        }
+
+        t("foo", "foo");
+
+        // Escaping of non-identifier characters like `=`.
+        t("operator==", "operator_3dh_3dh");
+
+        // Feeble attempt of testing injectivity
+        // (need to escape `_` to avoid a conflict with result of the previous test).
+        t("operator_3dh_3dh", "operator_u3dh_u3dh");
+    }
+}

tests/ffi/lib.rs

@@ -234,6 +234,9 @@ pub mod ffi {
         #[rust_name = "str_overloaded_function"]
         fn cOverloadedFunction(x: &str) -> String;
 
+        #[cxx_name = "operator=="]
+        fn cpp_eq_operator(&self, other_n: usize) -> bool;
+
         #[namespace = "other"]
         fn ns_c_take_ns_shared(shared: AShared);
 
@@ -352,6 +355,9 @@ pub mod ffi {
         #[cxx_name = "rAliasedFunction"]
         fn r_aliased_function(x: i32) -> String;
 
+        #[cxx_name = "operator=="]
+        fn r_operator_eq_for_usize_operand(self: &R, rhs: usize) -> bool;
+
         #[Self = "Shared"]
         fn r_static_method_on_shared() -> usize;
 
@@ -473,6 +479,10 @@ impl R {
     fn r_static_method() -> usize {
         2024
     }
+
+    fn r_operator_eq_for_usize_operand(&self, rhs: usize) -> bool {
+        self.0 == rhs
+    }
 }
 
 pub struct Reference<'a>(pub &'a String);