Initialize struct literal with list-initializer for C++11 standard (#401

) * Emit struct literal with list-initializer when language is Cxx * Update test results * Add test case - struct literal order * Memoize struct field names * Specify struct type at front of initializer
mozilla · Nov 17, 2019 · 3f9e54b · 3f9e54b
1 parent 5b4cda0
commit 3f9e54b
Show file tree

Hide file tree

Showing 15 changed files with 346 additions and 55 deletions.
diff --git a/src/bindgen/bindings.rs b/src/bindgen/bindings.rs
@@ -2,10 +2,13 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+use std::cell::RefCell;
+use std::collections::HashMap;
 use std::fs;
 use std::fs::File;
 use std::io::{Read, Write};
 use std::path;
+use std::rc::Rc;
 
 use bindgen::config::{Config, Language};
 use bindgen::ir::{
@@ -19,6 +22,7 @@ pub struct Bindings {
     /// The map from path to struct, used to lookup whether a given type is a
     /// transparent struct. This is needed to generate code for constants.
     struct_map: ItemMap<Struct>,
+    struct_fileds_memo: RefCell<HashMap<BindgenPath, Rc<Vec<String>>>>,
     globals: Vec<Static>,
     constants: Vec<Constant>,
     items: Vec<ItemContainer>,
@@ -43,6 +47,7 @@ impl Bindings {
         Bindings {
             config,
             struct_map,
+            struct_fileds_memo: Default::default(),
             globals,
             constants,
             items,
@@ -63,6 +68,30 @@ impl Bindings {
         any
     }
 
+    pub fn struct_field_names(&self, path: &BindgenPath) -> Rc<Vec<String>> {
+        let mut memos = self.struct_fileds_memo.borrow_mut();
+        if let Some(memo) = memos.get(path) {
+            return memo.clone();
+        }
+
+        let mut fields = Vec::<String>::new();
+        self.struct_map.for_items(path, |st| {
+            let mut pos: usize = 0;
+            for field in &st.fields {
+                if let Some(found_pos) = fields.iter().position(|v| *v == field.0) {
+                    pos = found_pos + 1;
+                } else {
+                    fields.insert(pos, field.0.clone());
+                    pos += 1;
+                }
+            }
+        });
+
+        let fields = Rc::new(fields);
+        memos.insert(path.clone(), fields.clone());
+        fields
+    }
+
     pub fn write_to_file<P: AsRef<path::Path>>(&self, path: P) -> bool {
         // Don't compare files if we've never written this file before
         if !path.as_ref().is_file() {

diff --git a/src/bindgen/ir/constant.rs b/src/bindgen/ir/constant.rs
@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use std::borrow::Cow;
-use std::fmt;
+use std::collections::HashMap;
 use std::io::Write;
 
 use syn;
@@ -23,6 +23,10 @@ use syn::UnOp;
 #[derive(Debug, Clone)]
 pub enum Literal {
     Expr(String),
+    PostfixUnaryOp {
+        op: &'static str,
+        value: Box<Literal>,
+    },
     BinOp {
         left: Box<Literal>,
         op: &'static str,
@@ -31,14 +35,14 @@ pub enum Literal {
     Struct {
         path: Path,
         export_name: String,
-        fields: Vec<(String, Literal)>,
+        fields: HashMap<String, Literal>,
     },
 }
 
 impl Literal {
     fn replace_self_with(&mut self, self_ty: &Path) {
         match *self {
-            Literal::BinOp { .. } | Literal::Expr(..) => {}
+            Literal::PostfixUnaryOp { .. } | Literal::BinOp { .. } | Literal::Expr(..) => {}
             Literal::Struct {
                 ref mut path,
                 ref mut export_name,
@@ -47,7 +51,7 @@ impl Literal {
                 if path.replace_self_with(self_ty) {
                     *export_name = self_ty.name().to_owned();
                 }
-                for &mut (ref _name, ref mut expr) in fields {
+                for (ref _name, ref mut expr) in fields {
                     expr.replace_self_with(self_ty);
                 }
             }
@@ -57,6 +61,7 @@ impl Literal {
     fn is_valid(&self, bindings: &Bindings) -> bool {
         match *self {
             Literal::Expr(..) => true,
+            Literal::PostfixUnaryOp { ref value, .. } => value.is_valid(bindings),
             Literal::BinOp {
                 ref left,
                 ref right,
@@ -67,33 +72,6 @@ impl Literal {
     }
 }
 
-impl fmt::Display for Literal {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self {
-            Literal::Expr(v) => write!(f, "{}", v),
-            Literal::BinOp {
-                ref left,
-                op,
-                ref right,
-            } => write!(f, "{} {} {}", left, op, right),
-            Literal::Struct {
-                export_name,
-                fields,
-                ..
-            } => write!(
-                f,
-                "({}){{ {} }}",
-                export_name,
-                fields
-                    .iter()
-                    .map(|(key, lit)| format!(".{} = {}", key, lit))
-                    .collect::<Vec<String>>()
-                    .join(", "),
-            ),
-        }
-    }
-}
-
 impl Literal {
     pub fn rename_for_config(&mut self, config: &Config) {
         match self {
@@ -107,6 +85,9 @@ impl Literal {
                     lit.rename_for_config(config);
                 }
             }
+            Literal::PostfixUnaryOp { ref mut value, .. } => {
+                value.rename_for_config(config);
+            }
             Literal::BinOp {
                 ref mut left,
                 ref mut right,
@@ -174,34 +155,88 @@ impl Literal {
                 ..
             }) => {
                 let struct_name = path.segments[0].ident.to_string();
-                let mut field_pairs: Vec<(String, Literal)> = Vec::new();
+                let mut field_map = HashMap::<String, Literal>::default();
                 for field in fields {
                     let ident = match field.member {
                         syn::Member::Named(ref name) => name.to_string(),
                         syn::Member::Unnamed(ref index) => format!("_{}", index.index),
                     };
                     let key = ident.to_string();
                     let value = Literal::load(&field.expr)?;
-                    field_pairs.push((key, value));
+                    field_map.insert(key, value);
                 }
                 Ok(Literal::Struct {
                     path: Path::new(struct_name.clone()),
                     export_name: struct_name,
-                    fields: field_pairs,
+                    fields: field_map,
                 })
             }
             syn::Expr::Unary(syn::ExprUnary {
                 ref op, ref expr, ..
             }) => match *op {
                 UnOp::Neg(_) => {
                     let val = Self::load(expr)?;
-                    Ok(Literal::Expr(format!("-{}", val)))
+                    Ok(Literal::PostfixUnaryOp {
+                        op: "-",
+                        value: Box::new(val),
+                    })
                 }
                 _ => Err(format!("Unsupported Unary expression. {:?}", *op)),
             },
             _ => Err(format!("Unsupported literal expression. {:?}", *expr)),
         }
     }
+
+    fn write<F: Write>(&self, config: &Config, out: &mut SourceWriter<F>) {
+        match self {
+            Literal::Expr(v) => write!(out, "{}", v),
+            Literal::PostfixUnaryOp { op, ref value } => {
+                write!(out, "{}", op);
+                value.write(config, out);
+            }
+            Literal::BinOp {
+                ref left,
+                op,
+                ref right,
+            } => {
+                left.write(config, out);
+                write!(out, " {} ", op);
+                right.write(config, out);
+            }
+            Literal::Struct {
+                export_name,
+                fields,
+                path,
+            } => {
+                if config.language == Language::C {
+                    write!(out, "({})", export_name);
+                } else {
+                    write!(out, "{}", export_name);
+                }
+
+                write!(out, "{{ ");
+                let mut is_first_field = true;
+                // In C++, same order as defined is required.
+                let ordered_fields = out.bindings().struct_field_names(path);
+                for ordered_key in ordered_fields.iter() {
+                    if let Some(ref lit) = fields.get(ordered_key) {
+                        if !is_first_field {
+                            write!(out, ", ");
+                        } else {
+                            is_first_field = false;
+                        }
+                        if config.language == Language::Cxx {
+                            write!(out, "/* .{} = */ ", ordered_key);
+                        } else {
+                            write!(out, ".{} = ", ordered_key);
+                        }
+                        lit.write(config, out);
+                    }
+                }
+                write!(out, " }}");
+            }
+        }
+    }
 }
 
 #[derive(Debug, Clone)]
@@ -405,7 +440,7 @@ impl Constant {
                 ref fields,
                 ref path,
                 ..
-            } if out.bindings().struct_is_transparent(path) => &fields[0].1,
+            } if out.bindings().struct_is_transparent(path) => &fields.iter().next().unwrap().1,
             _ => &self.value,
         };
 
@@ -417,9 +452,12 @@ impl Constant {
                 out.write("const ");
             }
             self.ty.write(config, out);
-            write!(out, " {} = {};", name, value)
+            write!(out, " {} = ", name);
+            value.write(config, out);
+            write!(out, ";");
         } else {
-            write!(out, "#define {} {}", name, value)
+            write!(out, "#define {} ", name);
+            value.write(config, out);
         }
         condition.write_after(config, out);
     }

diff --git a/tests/expectations/associated_in_body.cpp b/tests/expectations/associated_in_body.cpp
@@ -32,11 +32,11 @@ struct StyleAlignFlags {
   static const StyleAlignFlags END;
   static const StyleAlignFlags FLEX_START;
 };
-inline const StyleAlignFlags StyleAlignFlags::AUTO = (StyleAlignFlags){ .bits = 0 };
-inline const StyleAlignFlags StyleAlignFlags::NORMAL = (StyleAlignFlags){ .bits = 1 };
-inline const StyleAlignFlags StyleAlignFlags::START = (StyleAlignFlags){ .bits = 1 << 1 };
-inline const StyleAlignFlags StyleAlignFlags::END = (StyleAlignFlags){ .bits = 1 << 2 };
-inline const StyleAlignFlags StyleAlignFlags::FLEX_START = (StyleAlignFlags){ .bits = 1 << 3 };
+inline const StyleAlignFlags StyleAlignFlags::AUTO = StyleAlignFlags{ /* .bits = */ 0 };
+inline const StyleAlignFlags StyleAlignFlags::NORMAL = StyleAlignFlags{ /* .bits = */ 1 };
+inline const StyleAlignFlags StyleAlignFlags::START = StyleAlignFlags{ /* .bits = */ 1 << 1 };
+inline const StyleAlignFlags StyleAlignFlags::END = StyleAlignFlags{ /* .bits = */ 1 << 2 };
+inline const StyleAlignFlags StyleAlignFlags::FLEX_START = StyleAlignFlags{ /* .bits = */ 1 << 3 };
 
 extern "C" {
 

diff --git a/tests/expectations/bitflags.cpp b/tests/expectations/bitflags.cpp
@@ -27,11 +27,11 @@ struct AlignFlags {
     return *this;
   }
 };
-static const AlignFlags AlignFlags_AUTO = (AlignFlags){ .bits = 0 };
-static const AlignFlags AlignFlags_NORMAL = (AlignFlags){ .bits = 1 };
-static const AlignFlags AlignFlags_START = (AlignFlags){ .bits = 1 << 1 };
-static const AlignFlags AlignFlags_END = (AlignFlags){ .bits = 1 << 2 };
-static const AlignFlags AlignFlags_FLEX_START = (AlignFlags){ .bits = 1 << 3 };
+static const AlignFlags AlignFlags_AUTO = AlignFlags{ /* .bits = */ 0 };
+static const AlignFlags AlignFlags_NORMAL = AlignFlags{ /* .bits = */ 1 };
+static const AlignFlags AlignFlags_START = AlignFlags{ /* .bits = */ 1 << 1 };
+static const AlignFlags AlignFlags_END = AlignFlags{ /* .bits = */ 1 << 2 };
+static const AlignFlags AlignFlags_FLEX_START = AlignFlags{ /* .bits = */ 1 << 3 };
 
 extern "C" {
 

diff --git a/tests/expectations/both/struct_literal_order.c b/tests/expectations/both/struct_literal_order.c
@@ -0,0 +1,24 @@
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+typedef struct ABC {
+  float a;
+  uint32_t b;
+  uint32_t c;
+} ABC;
+#define ABC_abc (ABC){ .a = 1.0, .b = 2, .c = 3 }
+#define ABC_bac (ABC){ .a = 1.0, .b = 2, .c = 3 }
+#define ABC_cba (ABC){ .a = 1.0, .b = 2, .c = 3 }
+
+typedef struct BAC {
+  uint32_t b;
+  float a;
+  int32_t c;
+} BAC;
+#define BAC_abc (BAC){ .b = 1, .a = 2.0, .c = 3 }
+#define BAC_bac (BAC){ .b = 1, .a = 2.0, .c = 3 }
+#define BAC_cba (BAC){ .b = 1, .a = 2.0, .c = 3 }
+
+void root(ABC a1, BAC a2);
diff --git a/tests/expectations/both/struct_literal_order.compat.c b/tests/expectations/both/struct_literal_order.compat.c
@@ -0,0 +1,32 @@
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+typedef struct ABC {
+  float a;
+  uint32_t b;
+  uint32_t c;
+} ABC;
+#define ABC_abc (ABC){ .a = 1.0, .b = 2, .c = 3 }
+#define ABC_bac (ABC){ .a = 1.0, .b = 2, .c = 3 }
+#define ABC_cba (ABC){ .a = 1.0, .b = 2, .c = 3 }
+
+typedef struct BAC {
+  uint32_t b;
+  float a;
+  int32_t c;
+} BAC;
+#define BAC_abc (BAC){ .b = 1, .a = 2.0, .c = 3 }
+#define BAC_bac (BAC){ .b = 1, .a = 2.0, .c = 3 }
+#define BAC_cba (BAC){ .b = 1, .a = 2.0, .c = 3 }
+
+#ifdef __cplusplus
+extern "C" {
+#endif // __cplusplus
+
+void root(ABC a1, BAC a2);
+
+#ifdef __cplusplus
+} // extern "C"
+#endif // __cplusplus
diff --git a/tests/expectations/prefixed_struct_literal.cpp b/tests/expectations/prefixed_struct_literal.cpp
@@ -7,9 +7,9 @@ struct PREFIXFoo {
   int32_t a;
   uint32_t b;
 };
-static const PREFIXFoo PREFIXFoo_FOO = (PREFIXFoo){ .a = 42, .b = 47 };
+static const PREFIXFoo PREFIXFoo_FOO = PREFIXFoo{ /* .a = */ 42, /* .b = */ 47 };
 
-static const PREFIXFoo PREFIXBAR = (PREFIXFoo){ .a = 42, .b = 1337 };
+static const PREFIXFoo PREFIXBAR = PREFIXFoo{ /* .a = */ 42, /* .b = */ 1337 };
 
 extern "C" {
 

diff --git a/tests/expectations/prefixed_struct_literal_deep.cpp b/tests/expectations/prefixed_struct_literal_deep.cpp
@@ -13,7 +13,7 @@ struct PREFIXFoo {
   PREFIXBar bar;
 };
 
-static const PREFIXFoo PREFIXVAL = (PREFIXFoo){ .a = 42, .b = 1337, .bar = (PREFIXBar){ .a = 323 } };
+static const PREFIXFoo PREFIXVAL = PREFIXFoo{ /* .a = */ 42, /* .b = */ 1337, /* .bar = */ PREFIXBar{ /* .a = */ 323 } };
 
 extern "C" {