From 8151a20fac0ed955838df99cc830ebe4f8970dee Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 16 Jul 2019 10:03:10 -0400
Subject: [PATCH 090/108] FIXME: cleanups

---
 gcc/analyzer/constraint-manager.cc | 53 ++++++++++++++++++++++++--------------
 gcc/analyzer/constraint-manager.h  |  2 +-
 gcc/analyzer/impl-4.cc             |  5 ++--
 gcc/analyzer/region-model.cc       |  8 +++---
 gcc/analyzer/region-model.h        |  6 ++---
 gcc/analyzer/tristate.cc           | 15 ++++++++++-
 6 files changed, 60 insertions(+), 29 deletions(-)

diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc
index 2ea3163..73eda94 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -155,7 +155,9 @@ range::to_str () const
   return result;  // TODO
 }
 
-/* FIXME.  */
+/* Determine if there is only one possible value for this range.
+   If so, return true and write the constant to *OUT.
+   Otherwise, return false.  */
 
 bool
 range::constrained_to_single_element (tree *out)
@@ -188,7 +190,7 @@ range::constrained_to_single_element (tree *out)
 
 /* class equiv_class.  */
 
-/* FIXME.  */
+/* equiv_class's default ctor.  */
 
 equiv_class::equiv_class ()
 : m_constant (NULL_TREE), m_cst_sid (svalue_id::null ()),
@@ -196,7 +198,7 @@ equiv_class::equiv_class ()
 {
 }
 
-/* FIXME.  */
+/* equiv_class's copy ctor.  */
 
 equiv_class::equiv_class (const equiv_class &other)
 : m_constant (other.m_constant), m_cst_sid (other.m_cst_sid),
@@ -528,7 +530,7 @@ equiv_class_id::to_str () const
 
 /* class constraint_manager.  */
 
-/* FIXME.  */
+/* constraint_manager's copy ctor.  */
 
 constraint_manager::constraint_manager (const constraint_manager &other)
 {
@@ -650,7 +652,7 @@ constraint_manager::to_str () const
   return pp_formatted_text (&pp);
 }
 
-/* FIXME.  */
+/* Dump a multiline representation of this constraint_manager to PP.  */
 
 void
 constraint_manager::dump_to_pp (pretty_printer *pp) const
@@ -676,7 +678,7 @@ constraint_manager::dump_to_pp (pretty_printer *pp) const
     }
 }
 
-/* FIXME.  */
+/* Dump a multiline representation of this constraint_manager to FP.  */
 
 void
 constraint_manager::dump (FILE *fp) const
@@ -689,7 +691,7 @@ constraint_manager::dump (FILE *fp) const
   pp_flush (&pp);
 }
 
-/* FIXME.  */
+/* Dump a multiline representation of this constraint_manager to stderr.  */
 
 DEBUG_FUNCTION void
 constraint_manager::dump () const
@@ -1423,7 +1425,8 @@ constraint_manager::canonicalize (unsigned num_svalue_ids)
 /* A concrete subclass of constraint_manager for use when
    merging two constraint_manager into a third constraint_manager,
    each of which has its own region_model.
-   Calls are delegated to the constraint_manager for the merged model.  */
+   Calls are delegated to the constraint_manager for the merged model,
+   and thus affect its region_model.  */
 
 class cleaned_constraint_manager : public constraint_manager
 {
@@ -1480,7 +1483,8 @@ private:
 /* Use MERGER to merge CM_A and CM_B into *OUT.
    If one thinks of a constraint_manager as a subset of N-dimensional
    space, this takes the union of the points of CM_A and CM_B, and
-   expresses that into *OUT.  */
+   expresses that into *OUT.  Alternatively, it can be thought of
+   as the intersection of the constraints.  */
 
 void
 constraint_manager::merge (const constraint_manager &cm_a,
@@ -1517,7 +1521,8 @@ constraint_manager::merge (const constraint_manager &cm_a,
   cleaned_cm_a.for_each_fact (&v);
 }
 
-/* Use MAP_SID_TO_M to map equivalence classes and constraints from
+/* A subroutine of constraint_manager::merge.
+   Use MAP_SID_TO_M to map equivalence classes and constraints from
    SM_IN to *OUT.  Purge any non-constant svalue_id that don't appear
    in the result of MAP_SID_TO_M, purging any ECs and their constraints
    that become empty as a result.  Potentially create svalues in
@@ -1598,7 +1603,10 @@ constraint_manager::get_ec_map (auto_vec<equiv_class_id> *out_ec_for_sid) const
 }
 #endif
 
-/* FIXME.  */
+/* Call VISITOR's on_fact vfunc repeatedly to express the various
+   equivalence classes and constraints.
+   This is used by constraint_manager::merge to find the common
+   facts between two input constraint_managers.  */
 
 void
 constraint_manager::for_each_fact (fact_visitor *visitor) const
@@ -1646,7 +1654,7 @@ constraint_manager::for_each_fact (fact_visitor *visitor) const
     }
 }
 
-/* FIXME.  */
+/* Assert that this object is valid.  */
 
 void
 constraint_manager::validate () const
@@ -1748,7 +1756,8 @@ assert_condition (const location &loc,
 		    tristate (tristate::TS_UNKNOWN));		\
   SELFTEST_END_STMT
 
-/* FIXME.  */
+/* Verify that setting and getting simple conditions within a region_model
+   work (thus exercising the underlying constraint_manager).  */
 
 static void
 test_constraint_conditions ()
@@ -2010,7 +2019,7 @@ test_constraint_conditions ()
   // etc
 }
 
-/* FIXME.  */
+/* Test transitivity of conditions.  */
 
 static void
 test_transitivity ()
@@ -2156,7 +2165,8 @@ test_transitivity ()
 
 }
 
-/* FIXME.  */
+/* Test various conditionals involving constants where the results
+   ought to be implied based on the values of the constants.  */
 
 static void
 test_constant_comparisons ()
@@ -2248,18 +2258,23 @@ test_constant_comparisons ()
     }
   }
 
-#if 0
-  /* As above, but for float.   */
+  /* As above, but for floating-point:
+     if "f > 3" && "f <= 4" we don't know that f == 4.  */
   {
+    tree f = build_global_decl ("f", double_type_node);
+    tree float_3 = build_real_from_int_cst (double_type_node, int_3);
+    tree float_4 = build_real_from_int_cst (double_type_node, int_4);
+
     region_model model;
     ADD_SAT_CONSTRAINT (model, f, GT_EXPR, float_3);
     ADD_SAT_CONSTRAINT (model, f, LE_EXPR, float_4);
+    ASSERT_CONDITION_UNKNOWN (model, f, EQ_EXPR, float_4);
     ASSERT_CONDITION_UNKNOWN (model, f, EQ_EXPR, int_4);
   }
-#endif
 }
 
-/* FIXME.  */
+/* Verify various lower-level implementation details about
+   constraint_manager.  */
 
 static void
 test_constraint_impl ()
diff --git a/gcc/analyzer/constraint-manager.h b/gcc/analyzer/constraint-manager.h
index b2e290c..7d11e8b 100644
--- a/gcc/analyzer/constraint-manager.h
+++ b/gcc/analyzer/constraint-manager.h
@@ -75,7 +75,7 @@ public:
   bool m_escaped;
 };
 
-/* FIXME.  */
+/* The various kinds of constraint.  */
 
 enum constraint_op
 {
diff --git a/gcc/analyzer/impl-4.cc b/gcc/analyzer/impl-4.cc
index 95b77f0..9e44c3a 100644
--- a/gcc/analyzer/impl-4.cc
+++ b/gcc/analyzer/impl-4.cc
@@ -3750,11 +3750,13 @@ program_state::can_merge_with_p (const program_state &other,
   return true;
 }
 
-/* FIXME.  */
+/* Assert that this object is valid.  */
 
 void
 program_state::validate (const extrinsic_state &ext_state) const
 {
+  // TODO: skip this in a release build
+
   m_region_model->validate ();
   gcc_assert (m_checker_states.length () == ext_state.get_num_checkers ());
   int sm_idx;
@@ -3766,7 +3768,6 @@ program_state::validate (const extrinsic_state &ext_state) const
     }
 }
 
-
 ////////////////////////////////////////////////////////////////////////////
 
 /* FIXME.  */
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index f90c56c..7525c2b 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -2546,7 +2546,7 @@ region_model::dump () const
   dump (stderr);
 }
 
-/* FIXME.  */
+/* Assert that this object is valid.  */
 
 void
 region_model::validate () const
@@ -2660,7 +2660,8 @@ region_model::canonicalize (region_model_context *ctxt)
       c.walk_sid (*sid);
   }
 
-  /* (2): Walk all regions in a deterministic order.  */
+  /* (2): Walk all regions (and thus their values) in a deterministic
+     order.  */
   c.walk_rid (m_root_rid);
 
   /* (3): Ensure we've visited everything, as we don't want to purge
@@ -2676,7 +2677,8 @@ region_model::canonicalize (region_model_context *ctxt)
       c.walk_sid (svalue_id::from_int (i));
   }
 
-  /* (4): Apply the reordering to the regions and values.  */
+  /* (4): We now have a reordering of the regions and values.
+     Apply it.  */
   remap_svalue_ids (c.m_sid_map);
   remap_region_ids (c.m_rid_map);
   if (ctxt)
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 86edfa1..bc350e5 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -199,7 +199,7 @@ one_way_id_map<T>::get_dst_for_src (T src) const
   return m_src_to_dst[src.as_int ()];
 }
 
-/* FIXME.  */
+/* Dump this map to PP.  */
 
 template <typename T>
 inline void
@@ -221,7 +221,7 @@ one_way_id_map<T>::dump_to_pp (pretty_printer *pp) const
   pp_newline (pp);
 }
 
-/* FIXME.  */
+/* Dump this map to stderr.  */
 
 template <typename T>
 DEBUG_FUNCTION inline void
@@ -233,7 +233,7 @@ one_way_id_map<T>::dump () const
   pp_flush (&pp);
 }
 
-/* FIXME.  */
+/* Generate a std::string describing this map.  */
 
 template <typename T>
 inline std::string
diff --git a/gcc/analyzer/tristate.cc b/gcc/analyzer/tristate.cc
index e829cd5..ac16129 100644
--- a/gcc/analyzer/tristate.cc
+++ b/gcc/analyzer/tristate.cc
@@ -114,7 +114,8 @@ namespace selftest {
   ASSERT_EQ (TRISTATE, tristate (tristate::TS_UNKNOWN));	\
   SELFTEST_END_STMT
 
-/* FIXME.  */
+/* Test tristate's ctors, along with is_*, as_string, operator==, and
+   operator!=.  */
 
 static void
 test_ctors ()
@@ -143,6 +144,18 @@ test_ctors ()
   ASSERT_NE (u, t);
   ASSERT_NE (u, f);
   ASSERT_NE (t, f);
+
+  tristate t2 (true);
+  ASSERT_TRUE (t2.is_true ());
+  ASSERT_EQ (t, t2);
+
+  tristate f2 (false);
+  ASSERT_TRUE (f2.is_false ());
+  ASSERT_EQ (f, f2);
+
+  tristate u2 (tristate::unknown ());
+  ASSERT_TRUE (!u2.is_known ());
+  ASSERT_EQ (u, u2);
 }
 
 /* Test && on tristate instances.  */
-- 
1.8.5.3

