loop_analysis.h refactor

Remove redundant P template param.
Move the concept of a loop template that calls back to its base analysis into its own class.

Author: JohnDumbell

src/analyses/lexical_loops.h

@@ -61,9 +61,9 @@ Author: Diffblue Ltd
 ///   * [function] get_target() which returns an object that needs:
 ///     * [field] location_number which is an unsigned int.
 template <class P, class T>
-class lexical_loops_templatet : public loop_analysist<P, T>
+class lexical_loops_templatet : public loop_analysist<T>
 {
-  typedef loop_analysist<P, T> parentt;
+  typedef loop_analysist<T> parentt;
 
 public:
   typedef typename parentt::loopt lexical_loopt;
@@ -92,11 +92,13 @@ class lexical_loops_templatet : public loop_analysist<P, T>
       out << "Note not all loops were in lexical loop form\n";
   }
 
+  virtual ~lexical_loops_templatet() = default;
+
 protected:
   void compute(P &program);
   bool compute_lexical_loop(T, T);
 
-  bool all_in_lexical_loop_form;
+  bool all_in_lexical_loop_form = false;
 };
 
 typedef lexical_loops_templatet<
@@ -178,7 +180,7 @@ bool lexical_loops_templatet<P, T>::compute_lexical_loop(
   }
 
   auto insert_result = parentt::loop_map.emplace(
-    loop_head, lexical_loopt{*this, std::move(loop_instructions)});
+    loop_head, lexical_loopt{std::move(loop_instructions)});
 
   // If this isn't a new loop head (i.e. return_result.second is false) then we
   // have multiple backedges targeting one loop header: this is not in simple

src/analyses/loop_analysis.h

@@ -13,47 +13,33 @@ Author: Diffblue Ltd
 #ifndef CPROVER_ANALYSES_LOOP_ANALYSIS_H
 #define CPROVER_ANALYSES_LOOP_ANALYSIS_H
 
-template <class P, class T>
+#include <memory>
+
+template <class T>
 class loop_analysist;
 
 /// A loop, specified as a set of instructions
-template <class P, class T>
+template <class T>
 class loop_templatet
 {
-  typedef loop_analysist<P, T> parent_analysist;
   typedef std::set<T> loop_instructionst;
   loop_instructionst loop_instructions;
 
-  friend loop_analysist<P, T>;
+  friend loop_analysist<T>;
 
 public:
-  explicit loop_templatet(parent_analysist &loop_analysis)
-    : loop_analysis(loop_analysis)
-  {
-  }
+  loop_templatet() = default;
 
   template <typename InstructionSet>
-  loop_templatet(parent_analysist &loop_analysis, InstructionSet &&instructions)
-    : loop_instructions(std::forward<InstructionSet>(instructions)),
-      loop_analysis(loop_analysis)
+  explicit loop_templatet(InstructionSet &&instructions)
+    : loop_instructions(std::forward<InstructionSet>(instructions))
   {
   }
 
   /// Returns true if \p instruction is in this loop
-  bool contains(const T instruction) const
-  {
-    return loop_analysis.loop_contains(*this, instruction);
-  }
-
-  /// Get the \ref parent_analysist analysis this loop relates to
-  const parent_analysist &get_loop_analysis() const
-  {
-    return loop_analysis;
-  }
-  /// Get the \ref parent_analysist analysis this loop relates to
-  parent_analysist &get_loop_analysis()
+  bool virtual contains(const T instruction) const
   {
-    return loop_analysis;
+    return !loop_instructions.empty() && loop_instructions.count(instruction);
   }
 
   // NOLINTNEXTLINE(readability/identifiers)
@@ -83,58 +69,105 @@ class loop_templatet
     return loop_instructions.empty();
   }
 
-  /// Adds \p instruction to this loop. The caller must verify that the added
-  /// instruction does not alter loop structure; if it does they must discard
-  /// and recompute the related \ref parent_analysist instance.
+  /// Adds \p instruction to this loop.
   /// \return true if the instruction is new
   bool insert_instruction(const T instruction)
   {
     return loop_instructions.insert(instruction).second;
   }
-
-private:
-  parent_analysist &loop_analysis;
 };
 
-template <class P, class T>
+template <class T>
 class loop_analysist
 {
 public:
-  typedef loop_templatet<P, T> loopt;
+  typedef loop_templatet<T> loopt;
   // map loop headers to loops
   typedef std::map<T, loopt> loop_mapt;
 
   loop_mapt loop_map;
 
-  void output(std::ostream &) const;
+  virtual void output(std::ostream &) const;
+
+  /// Returns true if \p instruction is the header of any loop
+  bool is_loop_header(const T instruction) const
+  {
+    return loop_map.count(instruction);
+  }
+
+  loop_analysist() = default;
+};
+
+template <typename T>
+class loop_with_parent_analysis_templatet : loop_templatet<T>
+{
+  typedef loop_analysist<T> parent_analysist;
+
+public:
+  explicit loop_with_parent_analysis_templatet(parent_analysist &loop_analysis)
+    : loop_analysis(loop_analysis)
+  {
+  }
+
+  template <typename InstructionSet>
+  explicit loop_with_parent_analysis_templatet(
+    parent_analysist &loop_analysis,
+    InstructionSet &&instructions)
+    : loop_templatet<T>(std::forward<InstructionSet>(instructions)),
+      loop_analysis(loop_analysis)
+  {
+  }
 
   /// Returns true if \p instruction is in \p loop
-  bool loop_contains(const loopt &loop, const T instruction) const
+  bool loop_contains(
+    const typename loop_analysist<T>::loopt &loop,
+    const T instruction) const
   {
     return loop.loop_instructions.count(instruction);
   }
 
-  /// Returns true if \p instruction is the header of any loop
-  bool is_loop_header(const T instruction) const
+  /// Get the \ref parent_analysist analysis this loop relates to
+  const parent_analysist &get_loop_analysis() const
   {
-    return loop_map.count(instruction);
+    return loop_analysis;
+  }
+  /// Get the \ref parent_analysist analysis this loop relates to
+  parent_analysist &get_loop_analysis()
+  {
+    return loop_analysis;
   }
 
-  loop_analysist() = default;
+private:
+  parent_analysist &loop_analysis;
+};
+
+template <class T>
+class linked_loop_analysist : loop_analysist<T>
+{
+public:
+  linked_loop_analysist() = default;
+
+  /// Returns true if \p instruction is in \p loop
+  bool loop_contains(
+    const typename loop_analysist<T>::loopt &loop,
+    const T instruction) const
+  {
+    return loop.loop_instructions.count(instruction);
+  }
 
   // The loop structures stored in `loop_map` contain back-pointers to this
   // class, so we forbid copying or moving the analysis struct. If this becomes
   // necessary then either add a layer of indirection or update the loop_map
   // back-pointers on copy/move.
-  loop_analysist(const loop_analysist &) = delete;
-  loop_analysist(loop_analysist &&) = delete;
-  loop_analysist &operator=(const loop_analysist &) = delete;
-  loop_analysist &operator=(loop_analysist &&) = delete;
+  linked_loop_analysist(const linked_loop_analysist &) = delete;
+  linked_loop_analysist(linked_loop_analysist &&) = delete;
+  linked_loop_analysist &operator=(const linked_loop_analysist &) = delete;
+  linked_loop_analysist &operator=(linked_loop_analysist &&) = delete;
 };
 
 /// Print all natural loops that were found
-template <class P, class T>
-void loop_analysist<P, T>::output(std::ostream &out) const
+template <class T>
+void loop_analysist<T>::output(std::ostream &out) const
 {
   for(const auto &loop : loop_map)
   {

src/analyses/natural_loops.h

@@ -44,9 +44,9 @@ Author: Georg Weissenbacher, georg@weissenbacher.name
 ///   * [function] get_target() which returns an object that needs:
 ///     * [field] location_number which is an unsigned int.
 template <class P, class T>
-class natural_loops_templatet : public loop_analysist<P, T>
+class natural_loops_templatet : public loop_analysist<T>
 {
-  typedef loop_analysist<P, T> parentt;
+  typedef loop_analysist<T> parentt;
 
 public:
   typedef typename parentt::loopt natural_loopt;
@@ -140,7 +140,7 @@ void natural_loops_templatet<P, T>::compute_natural_loop(T m, T n)
 
   std::stack<T> stack;
 
-  auto insert_result = parentt::loop_map.emplace(n, natural_loopt{*this});
+  auto insert_result = parentt::loop_map.emplace(n, natural_loopt{});
   // Note the emplace *may* access a loop that already exists: this happens when
   // a given header has more than one incoming edge, such as
   // head: if(x) goto head; else goto head;

src/goto-instrument/loop_utils.h

@@ -17,7 +17,7 @@ Author: Daniel Kroening, kroening@kroening.com
 class local_may_aliast;
 
 typedef std::set<exprt> modifiest;
-typedef const natural_loops_mutablet::natural_loopt loopt;
+typedef natural_loops_mutablet::natural_loopt loopt;
 
 void get_modifies(
   const local_may_aliast &local_may_alias,