Skip to content

No fixpoint again + other issues #1376

@fitzgen

Description

@fitzgen

Steps to reproduce

  1. Checkout https://github.com/fitzgen/rust-bindgen/tree/414442638a913bf5d924f9b16b2ae417cde5840b (apologies for the large test case, I wasn't able to reproduce in a lib.rs outside of full bindgen, so I couldn't creduce it)

  2. Run cargo fmt

  3. Commit changes

  4. Re-run cargo fmt

Expected Results

The second cargo fmt does not result in any diff.

Actual Results

After the second cargo fmt I get this diff from the first cargo fmt:

modified   src/ir/context.rs
@@ -841,21 +841,20 @@ impl<'ctx> BindgenContext<'ctx> {
             // instantiating, we'll need to construct it ourselves. However,
             // there is an extra `NamespaceRef, NamespaceRef, ..., TemplateRef`
             // representing a reference to the outermost template declaration
             // that we need to filter out of the children. We need to do this
             // filtering because we already know which template declaration is
             // being specialized via the `location`'s type, and if we do not
             // filter it out, we'll add an extra layer of template instantiation
             // on accident.
-            let idx =
-                children.iter().position(|c| {
-                                             c.kind() ==
+            let idx = children.iter().position(|c| {
+                                                   c.kind() ==
                                              clang_sys::CXCursor_TemplateRef
-                                         });
+                                               });
             if let Some(idx) = idx {
                 if children.iter()
                     .take(idx)
                     .all(|c| c.kind() == clang_sys::CXCursor_NamespaceRef) {
                     children = children.into_iter().skip(idx + 1).collect();
                 }
             }
         }
modified   src/ir/item.rs
@@ -687,17 +687,19 @@ impl Item {
                 module.name()
                     .map(ToOwned::to_owned)
                     .unwrap_or_else(|| {
                         format!("_bindgen_mod_{}", self.exposed_id(ctx))
                     })
             }
             ItemKind::Type(ref ty) => {
                 let name = match *ty.kind() {
-                    TypeKind::ResolvedTypeRef(..) => panic!("should have resolved this in name_target()"),
+                    TypeKind::ResolvedTypeRef(..) => {
+                        panic!("should have resolved this in name_target()")
+                    }
                     _ => ty.name(),
                 };
                 name.map(ToOwned::to_owned)
                     .unwrap_or_else(|| {
                         format!("_bindgen_ty_{}", self.exposed_id(ctx))
                     })
             }
             ItemKind::Function(ref fun) => {
modified   src/lib.rs
@@ -767,27 +767,27 @@ impl<'ctx> Bindings<'ctx> {
             self.write(ref_writer).expect("Could not write bindings to string");
         }
         String::from_utf8(mod_str).unwrap()
     }
 
     /// Write these bindings as source text to a file.
     pub fn write_to_file<P: AsRef<Path>>(&self, path: P) -> io::Result<()> {
         let file = try!(OpenOptions::new()
-            .write(true)
-            .truncate(true)
-            .create(true)
-            .open(path));
+                            .write(true)
+                            .truncate(true)
+                            .create(true)
+                            .open(path));
         self.write(Box::new(file))
     }
 
     /// Write these bindings as source text to the given `Write`able.
     pub fn write<'a>(&self, mut writer: Box<Write + 'a>) -> io::Result<()> {
         try!(writer.write("/* automatically generated by rust-bindgen */\n\n"
-            .as_bytes()));
+                              .as_bytes()));
 
         for line in self.context
                 .options()
                 .raw_lines
                 .iter() {
             try!(writer.write(line.as_bytes()));
             try!(writer.write("\n".as_bytes()));
         }
@@ -808,20 +808,20 @@ impl<'ctx> Bindings<'ctx> {
     /// Generate and write dummy uses of all the types we parsed, if we've been
     /// requested to do so in the options.
     ///
     /// See the `uses` module for more information.
     pub fn write_dummy_uses(&mut self) -> io::Result<()> {
         let file = if let Some(ref dummy_path) =
             self.context.options().dummy_uses {
             Some(try!(OpenOptions::new()
-                .write(true)
-                .truncate(true)
-                .create(true)
-                .open(dummy_path)))
+                          .write(true)
+                          .truncate(true)
+                          .create(true)
+                          .open(dummy_path)))
         } else {
             None
         };
 
         if let Some(file) = file {
             try!(uses::generate_dummy_uses(&mut self.context, file));
         }

A third cargo fmt undoes the changes to build.rs.

After that, repeatedly running cargo fmt will repeatedly create and undo the same formatting changes to src/ir/item.rs. Although -- and this is really bizarre* -- sometimes it take a couple invocations of cargo fmt before the formatting to src/ir/item.rs re-applied or unapplied. This is the diff that gets applied and unapplied:

modified   src/ir/item.rs
@@ -687,17 +687,19 @@ impl Item {
                 module.name()
                     .map(ToOwned::to_owned)
                     .unwrap_or_else(|| {
                         format!("_bindgen_mod_{}", self.exposed_id(ctx))
                     })
             }
             ItemKind::Type(ref ty) => {
                 let name = match *ty.kind() {
-                    TypeKind::ResolvedTypeRef(..) => panic!("should have resolved this in name_target()"),
+                    TypeKind::ResolvedTypeRef(..) => {
+                        panic!("should have resolved this in name_target()")
+                    }
                     _ => ty.name(),
                 };
                 name.map(ToOwned::to_owned)
                     .unwrap_or_else(|| {
                         format!("_bindgen_ty_{}", self.exposed_id(ctx))
                     })
             }
             ItemKind::Function(ref fun) => {

Additionally, as per rust-lang/rust-bindgen#575 (review), there is also:

  • Bogus formatting that a human would never create
  • Removal of trailing commas despite the rustfmt.toml requesting they always be present

See those comments for examples.


I'm using this version of rustfmt:

$ rustfmt --version
0.8.0 ()

cc @nrc

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: this is a bug; use also I-* labels for specific bug kinds, e.g. I-non-idempotency or I-ICEI-poor-formattingIssue: poor formatting

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions