Define a type for communicating that the constraint function hasn't reached a fixed point

fitzgen · fitzgen · commit 79a03e4a990e · 2017-07-20T15:36:25.000-07:00
`ConstrainResult::Changed` is much more legible than `true`.
diff --git a/src/ir/analysis/derive_debug.rs b/src/ir/analysis/derive_debug.rs
@@ -1,5 +1,6 @@
 //! Determining which types for which we can emit `#[derive(Debug)]`.
-use super::MonotoneFramework;
+
+use super::{ConstrainResult, MonotoneFramework};
 use ir::context::{BindgenContext, ItemId};
 use ir::item::ItemSet;
 use std::collections::HashSet;
@@ -77,15 +78,15 @@ impl<'ctx, 'gen> CannotDeriveDebug<'ctx, 'gen> {
         }
     }
 
-    fn insert(&mut self, id: ItemId) -> bool {
+    fn insert(&mut self, id: ItemId) -> ConstrainResult {
         let was_not_already_in_set = self.cannot_derive_debug.insert(id);
         assert!(
             was_not_already_in_set,
             "We shouldn't try and insert {:?} twice because if it was \
              already in the set, `constrain` should have exited early.",
             id
         );
-        true
+        ConstrainResult::Changed
     }
 }
 
@@ -137,20 +138,20 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
         self.ctx.whitelisted_items().iter().cloned().collect()
     }
 
-    fn constrain(&mut self, id: ItemId) -> bool {
+    fn constrain(&mut self, id: ItemId) -> ConstrainResult {
         if self.cannot_derive_debug.contains(&id) {
-            return false;
+            return ConstrainResult::Same;
         }
 
         let item = self.ctx.resolve_item(id);
         let ty = match item.as_type() {
-            None => return false,
+            None => return ConstrainResult::Same,
             Some(ty) => ty
         };
 
         match *ty.kind() {
             // Handle the simple cases. These can derive debug without further
-            // information
+            // information.
             TypeKind::Void |
             TypeKind::NullPtr |
             TypeKind::Int(..) |
@@ -165,96 +166,112 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
             TypeKind::ObjCInterface(..) |
             TypeKind::ObjCId |
             TypeKind::ObjCSel => {
-                return false;
+                ConstrainResult::Same
             },
+
             TypeKind::Opaque => {
                 if ty.layout(self.ctx)
                     .map_or(true, |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) {
-                        return false;
+                        ConstrainResult::Same
                     } else {
-                        return self.insert(id);
+                        self.insert(id)
                     }
             },
+
             TypeKind::Array(t, len) => {
+                if self.cannot_derive_debug.contains(&t) {
+                    return self.insert(id);
+                }
+
                 if len <= RUST_DERIVE_IN_ARRAY_LIMIT {
-                    if self.cannot_derive_debug.contains(&t) {
-                        return self.insert(id);
-                    }
-                    return false;
+                    ConstrainResult::Same
                 } else {
-                    return self.insert(id);
+                    self.insert(id)
                 }
             },
+
             TypeKind::ResolvedTypeRef(t) |
             TypeKind::TemplateAlias(t, _) |
             TypeKind::Alias(t) => {
                 if self.cannot_derive_debug.contains(&t) {
-                    return self.insert(id);
+                    self.insert(id)
+                } else {
+                    ConstrainResult::Same
                 }
-                return false;
             },
+
             TypeKind::Comp(ref info) => {
                 if info.has_non_type_template_params() {
-                    if ty.layout(self.ctx).map_or(true,
-                                                  |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) {
-                        return false;
+                    if ty.layout(self.ctx)
+                        .map_or(true,
+                                |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) {
+                        return ConstrainResult::Same;
                     } else {
                         return self.insert(id);
                     }
                 }
+
                 if info.kind() == CompKind::Union {
                     if self.ctx.options().unstable_rust {
                         return self.insert(id);
                     }
 
-                    if ty.layout(self.ctx).map_or(true,
-                                                  |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) {
-                        return false;
+                    if ty.layout(self.ctx)
+                        .map_or(true,
+                                |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) {
+                        return ConstrainResult::Same;
                     } else {
                         return self.insert(id);
                     }
                 }
+
                 let bases_cannot_derive = info.base_members()
                     .iter()
                     .any(|base| self.cannot_derive_debug.contains(&base.ty));
                 if bases_cannot_derive {
                     return self.insert(id);
                 }
+
                 let fields_cannot_derive = info.fields()
                     .iter()
                     .any(|f| {
-                        match f {
-                            &Field::DataMember(ref data) => self.cannot_derive_debug.contains(&data.ty()),
-                            &Field::Bitfields(ref bfu) => bfu.bitfields()
-                                .iter().any(|b| {
-                                    self.cannot_derive_debug.contains(&b.ty())
-                                })
+                        match *f {
+                            Field::DataMember(ref data) => {
+                                self.cannot_derive_debug.contains(&data.ty())
+                            }
+                            Field::Bitfields(ref bfu) => {
+                                bfu.bitfields()
+                                    .iter().any(|b| {
+                                        self.cannot_derive_debug.contains(&b.ty())
+                                    })
+                            }
                         }
                     });
                 if fields_cannot_derive {
                     return self.insert(id);
                 }
-                false
+
+                ConstrainResult::Same
             },
+
             TypeKind::Pointer(inner) => {
-                let inner_type = self.ctx.resolve_type(inner);
-                if let TypeKind::Function(ref sig) =
-                    *inner_type.canonical_type(self.ctx).kind() {
-                        if sig.can_trivially_derive_debug(&self.ctx, ()) {
-                            return false;
-                        } else {
-                            return self.insert(id);
-                        }
+                let inner_type = self.ctx.resolve_type(inner).canonical_type(self.ctx);
+                if let TypeKind::Function(ref sig) = *inner_type.kind() {
+                    if !sig.can_trivially_derive_debug(&self.ctx, ()) {
+                        return self.insert(id);
                     }
-                false
+                }
+                ConstrainResult::Same
             },
+
             TypeKind::TemplateInstantiation(ref template) => {
                 let args_cannot_derive = template.template_arguments()
                     .iter()
                     .any(|arg| self.cannot_derive_debug.contains(&arg));
                 if args_cannot_derive {
                     return self.insert(id);
                 }
+
                 let ty_cannot_derive = template.template_definition()
                     .into_resolver()
                     .through_type_refs()
@@ -269,19 +286,26 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
                         // idea of the layout than the definition does.
                         if c.has_non_type_template_params() {
                             let opaque = ty.layout(self.ctx)
-                                .or_else(|| self.ctx.resolve_type(template.template_definition()).layout(self.ctx))
+                                .or_else(|| {
+                                    self.ctx
+                                        .resolve_type(template.template_definition())
+                                        .layout(self.ctx)
+                                })
                                 .unwrap_or(Layout::zero())
                                 .opaque();
                             Some(!opaque.can_trivially_derive_debug(&self.ctx, ()))
                         } else {
                             None
                         }
                     })
-                    .unwrap_or_else(|| self.cannot_derive_debug.contains(&template.template_definition()));
+                    .unwrap_or_else(|| {
+                        self.cannot_derive_debug.contains(&template.template_definition())
+                    });
                 if ty_cannot_derive {
                     return self.insert(id);
                 }
-                false
+
+                ConstrainResult::Same
             },
         }
     }
diff --git a/src/ir/analysis/mod.rs b/src/ir/analysis/mod.rs
@@ -89,10 +89,10 @@ pub trait MonotoneFramework: Sized + fmt::Debug {
     ///
     /// If this results in changing our internal state (ie, we discovered that
     /// we have not reached a fix-point and iteration should continue), return
-    /// `true`. Otherwise, return `false`. When `constrain` returns false for
-    /// all nodes in the set, we have reached a fix-point and the analysis is
-    /// complete.
-    fn constrain(&mut self, node: Self::Node) -> bool;
+    /// `ConstrainResult::Changed`. Otherwise, return `ConstrainResult::Same`.
+    /// When `constrain` returns `ConstrainResult::Same` for all nodes in the
+    /// set, we have reached a fix-point and the analysis is complete.
+    fn constrain(&mut self, node: Self::Node) -> ConstrainResult;
 
     /// For each node `d` that depends on the given `node`'s current answer when
     /// running `constrain(d)`, call `f(d)`. This informs us which new nodes to
@@ -102,6 +102,17 @@ pub trait MonotoneFramework: Sized + fmt::Debug {
         where F: FnMut(Self::Node);
 }
 
+/// Whether an analysis's `constrain` function modified the incremental results
+/// or not.
+pub enum ConstrainResult {
+    /// The incremental results were updated, and the fix-point computation
+    /// should continue.
+    Changed,
+
+    /// The incremental results were not updated.
+    Same,
+}
+
 /// Run an analysis in the monotone framework.
 pub fn analyze<Analysis>(extra: Analysis::Extra) -> Analysis::Output
     where Analysis: MonotoneFramework,
@@ -110,7 +121,7 @@ pub fn analyze<Analysis>(extra: Analysis::Extra) -> Analysis::Output
     let mut worklist = analysis.initial_worklist();
 
     while let Some(node) = worklist.pop() {
-        if analysis.constrain(node) {
+        if let ConstrainResult::Changed = analysis.constrain(node) {
             analysis.each_depending_on(node, |needs_work| {
                 worklist.push(needs_work);
             });
@@ -227,7 +238,7 @@ mod tests {
             self.graph.0.keys().cloned().collect()
         }
 
-        fn constrain(&mut self, node: Node) -> bool {
+        fn constrain(&mut self, node: Node) -> ConstrainResult {
             // The set of nodes reachable from a node `x` is
             //
             //     reachable(x) = s_0 U s_1 U ... U reachable(s_0) U reachable(s_1) U ...
@@ -254,7 +265,11 @@ mod tests {
             }
 
             let new_size = self.reachable[&node].len();
-            original_size != new_size
+            if original_size != new_size {
+                ConstrainResult::Changed
+            } else {
+                ConstrainResult::Same
+            }
         }
 
         fn each_depending_on<F>(&self, node: Node, mut f: F)
diff --git a/src/ir/analysis/template_params.rs b/src/ir/analysis/template_params.rs
@@ -88,7 +88,7 @@
 //!
 //! See `src/ir/analysis.rs` for more.
 
-use super::MonotoneFramework;
+use super::{ConstrainResult, MonotoneFramework};
 use ir::context::{BindgenContext, ItemId};
 use ir::item::{Item, ItemSet};
 use ir::template::{TemplateInstantiation, TemplateParameters};
@@ -468,7 +468,7 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> {
             .collect()
     }
 
-    fn constrain(&mut self, id: ItemId) -> bool {
+    fn constrain(&mut self, id: ItemId) -> ConstrainResult {
         // Invariant: all hash map entries' values are `Some` upon entering and
         // exiting this method.
         extra_assert!(self.used.values().all(|v| v.is_some()));
@@ -520,7 +520,11 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> {
         self.used.insert(id, Some(used_by_this_id));
         extra_assert!(self.used.values().all(|v| v.is_some()));
 
-        new_len != original_len
+        if new_len != original_len {
+            ConstrainResult::Changed
+        } else {
+            ConstrainResult::Same
+        }
     }
 
     fn each_depending_on<F>(&self, item: ItemId, mut f: F)
