Remove Display impl because it violates the trait's contract.

Fixes #233

Author: khuey

README.md

@@ -4,9 +4,9 @@
 
 This crate can parse a C++ “mangled” linker symbol name into a Rust value
 describing what the name refers to: a variable, a function, a virtual table,
-etc. The description type implements `Display`, producing human-readable text
-describing the mangled name. Debuggers and profilers can use this crate to
-provide more meaningful output.
+etc. The description type implements functions such as `demangle()`,
+producing human-readable text describing the mangled name. Debuggers and
+profilers can use this crate to provide more meaningful output.
 
 C++ requires the compiler to choose names for linker symbols consistently across
 compilation units, so that two compilation units that have seen the same
@@ -24,9 +24,9 @@ For example, suppose a C++ compilation unit has the definition:
 
 The Itanium C++ ABI specifies that the linker symbol for that function must be
 named `_ZN5space3fooEii`. This crate can parse that name into a Rust value
-representing its structure. Formatting the value with the `format!` macro or the
-`std::string::ToString::to_string` trait method yields the string
-`space::foo(int, int)`, which is more meaningful to the C++ developer.
+representing its structure. That Rust value can be `demangle()`d to the
+string `space::foo(int, int)`, which is more meaningful to the C++
+developer.
 
 ## Usage
 
@@ -49,7 +49,7 @@ let mangled = b"_ZN5space3fooEibc";
 let sym = Symbol::new(&mangled[..])
     .expect("Could not parse mangled symbol!");
 
-let demangled = sym.to_string();
+let demangled = sym.demangle().unwrap();
 assert_eq!(demangled, "space::foo(int, bool, char)");
 ```
 

build.rs

@@ -126,7 +126,6 @@ fn generate_compatibility_tests_from_libiberty() -> io::Result<()> {
         "
 extern crate cpp_demangle;
 extern crate diff;
-use std::fmt::Write;
 "
     )?;
 
@@ -245,12 +244,14 @@ fn test_libiberty_demangle_{}_() {{
         Err(e) => panic!("Should parse mangled symbol {{}}", e),
     }};
 
-    let mut actual = String::new();
-    if let Err(e) = write!(&mut actual, "{{}}", sym) {{
-        panic!("Error while demangling '{{}}': {{}}",
-               mangled_str,
-               e);
-    }}
+    let actual = match sym.demangle() {{
+        Ok(a) => a,
+        Err(e) => {{
+            panic!("Error while demangling '{{}}': {{}}",
+                   mangled_str,
+                   e);
+        }}
+    }};
 
     println!("     Expect demangled symbol: {{}}", expected);
     println!("Actually demangled symbol as: {{}}", actual);
@@ -282,7 +283,7 @@ fn test_libiberty_demangle_{}_() {{
         println!("");
     }}
 
-    assert_eq!(expected, actual);
+    assert_eq!(*expected, actual);
 }}
 "###,
             cfg,

c_api/src/c_api.rs

@@ -15,7 +15,7 @@ pub unsafe extern "C" fn demangle(
     let buffer = ffi::CStr::from_ptr(buffer);
 
     if let Ok((symbol, _)) = cpp_demangle::BorrowedSymbol::with_tail_and_options(&buffer.to_bytes_with_nul(), &parse_options) {
-        return ffi::CString::new(symbol.demangle(&demangle_options).unwrap()).unwrap().into_raw();
+        return ffi::CString::new(symbol.demangle_with_options(&demangle_options).unwrap()).unwrap().into_raw();
     }
 
     buffer.to_owned().into_raw()

examples/cppfilt.rs

@@ -51,7 +51,7 @@ where
 
         if let Ok((sym, tail)) = BorrowedSymbol::with_tail(&line[idx..]) {
             let demangled = sym
-                .demangle(&options)
+                .demangle_with_options(&options)
                 .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
             write!(out, "{}", demangled)?;
             line = tail;

examples/simple.rs

@@ -4,7 +4,7 @@ pub fn main() -> Result<(), Box<dyn std::error::Error>> {
     let mangled = b"_ZN5space3fooEibc";
 
     let sym = Symbol::new(&mangled[..])?;
-    let demangled = sym.to_string();
+    let demangled = sym.demangle()?;
 
     println!("{}", demangled);
 

fuzz/fuzzers/cppfilt_differential.rs

@@ -52,7 +52,7 @@ fuzz_target!(|data: &[u8]| {
     }
 
     let sym = ignore_err!(cpp_demangle::Symbol::new(data));
-    let demangled = sym.to_string();
+    let demangled = sym.demangle().unwrap();
     assert_eq!(demangled.trim(), cppfilt_output.trim(),
                "Should demangle the input the same as `c++filt` does");
 });

fuzz/fuzzers/parse_and_stringify.rs

@@ -4,6 +4,6 @@ extern crate cpp_demangle;
 
 fuzz_target!(|data: &[u8]| {
     if let Ok(sym) = cpp_demangle::Symbol::new(data) {
-        let _ = sym.to_string();
+        let _ = sym.demangle().unwrap();
     }
 });

src/lib.rs

@@ -1,8 +1,8 @@
 //! This crate can parse a C++ “mangled” linker symbol name into a Rust value
 //! describing what the name refers to: a variable, a function, a virtual table,
-//! etc. The description type implements `Display`, producing human-readable
-//! text describing the mangled name. Debuggers and profilers can use this crate
-//! to provide more meaningful output.
+//! etc. The description type implements functions such as `demangle()`,
+//! producing human-readable text describing the mangled name. Debuggers and
+//! profilers can use this crate to provide more meaningful output.
 //!
 //! C++ requires the compiler to choose names for linker symbols consistently
 //! across compilation units, so that two compilation units that have seen the
@@ -22,8 +22,7 @@
 //!
 //! The Itanium C++ ABI specifies that the linker symbol for that function must
 //! be named `_ZN5space3fooEii`. This crate can parse that name into a Rust
-//! value representing its structure. Formatting the value with the `format!`
-//! macro or the `std::string::ToString::to_string` trait method yields the
+//! value representing its structure. That Rust value can be `demangle()`d to the
 //! string `space::foo(int, int)`, which is more meaningful to the C++
 //! developer.
 
@@ -151,7 +150,6 @@ where
     ///
     /// ```
     /// use cpp_demangle::Symbol;
-    /// use std::string::ToString;
     ///
     /// // First, something easy :)
     ///
@@ -160,7 +158,7 @@ where
     /// let sym = Symbol::new(&mangled[..])
     ///     .expect("Could not parse mangled symbol!");
     ///
-    /// let demangled = sym.to_string();
+    /// let demangled = sym.demangle().unwrap();
     /// assert_eq!(demangled, "space::foo(int, bool, char)");
     ///
     /// // Now let's try something a little more complicated!
@@ -171,7 +169,7 @@ where
     /// let sym = Symbol::new(&mangled[..])
     ///     .expect("Could not parse mangled symbol!");
     ///
-    /// let demangled = sym.to_string();
+    /// let demangled = sym.demangle().unwrap();
     /// assert_eq!(
     ///     demangled,
     ///     "JS_GetPropertyDescriptorById(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::PropertyDescriptor>)"
@@ -186,7 +184,6 @@ where
     ///
     /// ```
     /// use cpp_demangle::{ParseOptions, Symbol};
-    /// use std::string::ToString;
     ///
     /// // First, something easy :)
     ///
@@ -198,7 +195,7 @@ where
     /// let sym = Symbol::new_with_options(&mangled[..], &parse_options)
     ///     .expect("Could not parse mangled symbol!");
     ///
-    /// let demangled = sym.to_string();
+    /// let demangled = sym.demangle().unwrap();
     /// assert_eq!(demangled, "space::foo(int, bool, char)");
     ///
     /// // Now let's try something a little more complicated!
@@ -209,7 +206,7 @@ where
     /// let sym = Symbol::new(&mangled[..])
     ///     .expect("Could not parse mangled symbol!");
     ///
-    /// let demangled = sym.to_string();
+    /// let demangled = sym.demangle().unwrap();
     /// assert_eq!(
     ///     demangled,
     ///     "JS_GetPropertyDescriptorById(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::PropertyDescriptor>)"
@@ -252,27 +249,40 @@ substitutions = {:#?}",
         Ok(symbol)
     }
 
-    /// Demangle the symbol and return it as a String.
+    /// Demangle the symbol and return it as a String, with the default options.
+    ///
+    /// ```
+    /// use cpp_demangle::{DemangleOptions, Symbol};
     ///
-    /// Unlike the `ToString` implementation, this function allows options to
-    /// be specified.
+    /// let mangled = b"_ZN5space3fooEibc";
+    ///
+    /// let sym = Symbol::new(&mangled[..])
+    ///     .expect("Could not parse mangled symbol!");
+    ///
+    /// let demangled = sym.demangle().unwrap();
+    /// assert_eq!(demangled, "space::foo(int, bool, char)");
+    /// ```
+    #[inline]
+    pub fn demangle(&self) -> ::core::result::Result<String, fmt::Error> {
+        self.demangle_with_options(&Default::default())
+    }
+
+    /// Demangle the symbol and return it as a String.
     ///
     /// ```
     /// use cpp_demangle::{DemangleOptions, Symbol};
-    /// use std::string::ToString;
     ///
     /// let mangled = b"_ZN5space3fooEibc";
     ///
     /// let sym = Symbol::new(&mangled[..])
     ///     .expect("Could not parse mangled symbol!");
     ///
-    /// let demangled = sym.to_string();
     /// let options = DemangleOptions::default();
-    /// let demangled_again = sym.demangle(&options).unwrap();
-    /// assert_eq!(demangled_again, demangled);
+    /// let demangled = sym.demangle_with_options(&options).unwrap();
+    /// assert_eq!(demangled, "space::foo(int, bool, char)");
     /// ```
     #[allow(clippy::trivially_copy_pass_by_ref)]
-    pub fn demangle(
+    pub fn demangle_with_options(
         &self,
         options: &DemangleOptions,
     ) -> ::core::result::Result<String, fmt::Error> {
@@ -361,7 +371,6 @@ where
     ///
     /// ```
     /// use cpp_demangle::BorrowedSymbol;
-    /// use std::string::ToString;
     ///
     /// let mangled = b"_ZN5space3fooEibc and some trailing junk";
     ///
@@ -370,7 +379,7 @@ where
     ///
     /// assert_eq!(tail, b" and some trailing junk");
     ///
-    /// let demangled = sym.to_string();
+    /// let demangled = sym.demangle().unwrap();
     /// assert_eq!(demangled, "space::foo(int, bool, char)");
     /// ```
     #[inline]
@@ -387,7 +396,6 @@ where
     ///
     /// ```
     /// use cpp_demangle::{BorrowedSymbol, ParseOptions};
-    /// use std::string::ToString;
     ///
     /// let mangled = b"_ZN5space3fooEibc and some trailing junk";
     ///
@@ -399,7 +407,7 @@ where
     ///
     /// assert_eq!(tail, b" and some trailing junk");
     ///
-    /// let demangled = sym.to_string();
+    /// let demangled = sym.demangle().unwrap();
     /// assert_eq!(demangled, "space::foo(int, bool, char)");
     /// ```
     pub fn with_tail_and_options(
@@ -433,26 +441,3 @@ substitutions = {:#?}",
         Ok((symbol, tail.into()))
     }
 }
-
-impl<T> fmt::Display for Symbol<T>
-where
-    T: AsRef<[u8]>,
-{
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let mut out = String::new();
-        {
-            let options = DemangleOptions::default();
-            let mut ctx = ast::DemangleContext::new(
-                &self.substitutions,
-                self.raw.as_ref(),
-                options,
-                &mut out,
-            );
-            self.parsed.demangle(&mut ctx, None).map_err(|err| {
-                log!("Demangling error: {:#?}", err);
-                fmt::Error
-            })?;
-        }
-        write!(f, "{}", &out)
-    }
-}

tests/libxul.rs

@@ -45,7 +45,6 @@ fn libxul_symbols_demangle() {
     let mut log_file = BufWriter::new(log_file);
 
     let mut line = Vec::new();
-    let mut demangled = Vec::new();
     let mut libiberty_sym = Vec::new();
 
     let which_cppfilt = get_cppfilt();
@@ -86,8 +85,7 @@ fn libxul_symbols_demangle() {
             num_parsed += 1;
 
             // Demangle the symbol.
-            demangled.clear();
-            if write!(&mut demangled, "{}", sym).is_ok() {
+            if let Ok(demangled) = sym.demangle() {
                 num_demangled += 1;
 
                 // Finally, we are going to have `c++filt` demangle the
@@ -107,6 +105,7 @@ fn libxul_symbols_demangle() {
                     .expect("should read line from c++filt");
                 // Drop the "\n".
                 libiberty_sym.pop();
+                let libiberty_sym = String::from_utf8_lossy(&libiberty_sym);
 
                 // Compare our demangling to libiberty's.
                 if libiberty_sym == demangled {
@@ -121,13 +120,13 @@ fn libxul_symbols_demangle() {
                     writeln!(
                         &mut log_file,
                         "           ...we demangled to: {}",
-                        String::from_utf8_lossy(&demangled)
+                        demangled
                     )
                     .unwrap();
                     writeln!(
                         &mut log_file,
                         "    ...libiberty demangled to: {}",
-                        String::from_utf8_lossy(&libiberty_sym)
+                        libiberty_sym
                     )
                     .unwrap();
                 }