From 4f5ddc23f4f6fec400c00b9d32359ee028049f0e Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 23 Feb 2021 17:18:24 -0500
Subject: [PATCH 24/30] FIXME: move const-epath heuristic to a subroutine

---
 gcc/analyzer/diagnostic-manager.cc | 338 ++++++++++++++++-------------
 1 file changed, 184 insertions(+), 154 deletions(-)

diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index cb43ab2338a..e7524965ee0 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -84,28 +84,14 @@ public:
 				 feasibility_problem **out_problem);
 
 private:
+  exploded_path *get_best_concat_epath (const exploded_node *enode);
+
   const exploded_graph &m_eg;
   shortest_exploded_paths m_sep;
 };
 
 /* class epath_finder.  */
 
-/* FIXME.  */
-
-static bool
-interesting_p (const exploded_node *enode)
-{
-  if (enode->m_preds.length () > 1)
-    return true;
-  if (enode->m_preds.length () == 1)
-    {
-      const exploded_node *pred = enode->m_preds[0]->m_src;
-      if (pred->m_succs.length () > 1)
-	return true;
-    }
-  return false;
-}
-
 /* Get the "best" exploded_path for reaching ENODE from the origin,
    returning ownership of it to the caller.
 
@@ -157,174 +143,218 @@ epath_finder::get_best_epath (const exploded_node *enode,
       if (flag_analyzer_feasibility)
 	{
 	  if (logger)
-	    logger->log ("rejecting %qs at EN: %i, SN: %i"
-			 " due to infeasible path",
-			 desc, enode->m_index,
-			 snode_idx);
+	    logger->log ("%qs: shortest path to EN: %i is infeasible",
+			 desc, enode->m_index);
 	  delete epath;
 
-	  /* TODO: Try harder; might need to clobber/delete *out_problem.
-	     Might want a cleaner interface for this.  */
-
-	  /* Create trimmed graph for just ENODE.
-	     Iterate through modifications.
-	       Create mutated graph expressing modifications.
-	       Find shortest path through modification.
-	       Is it feasible?
-	         Consider it.
-	     Return the shortest feasible path that was found
-	     via modifications.
-	     But we'll also want to debug feasibility issues, I guess.
-	     Modifications: perhaps require visiting an enode first,
-	     for "interesting" enodes, where enodes are
-	     (i) ones that weren't in the epath we found earlier
-	     (ii) ones whose predecessor has more than one out-edge
-	     (i.e. there was a choice beforehand)
-	     Does that get us further?
-	     Would be interesting to list those enodes, when logging.
-	     (and maybe highlight them in a custom dump?)
-	     Do we actually have to build a trimmed graph, or merely build
-	     the subset of interest?
-	     (and trimming could be confusing when dealing with indexes).  */
-	  // FIXME: find "interesting" enodes:
-	  unsigned i;
-	  exploded_node *iter_enode;
-	  exploded_path *best_concat_epath = NULL;
-	  exploded_node *best_iter_enode = NULL;
-	  FOR_EACH_VEC_ELT (m_eg.m_nodes, i, iter_enode)
+	  /* Try harder.  */
+	  exploded_path *best_concat_epath = get_best_concat_epath (enode);
+	  if (best_concat_epath)
+	    {
+	      if (logger)
+		logger->log ("accepting %qs at EN: %i, SN: %i with feasible"
+			     " concat path",
+			     desc, enode->m_index,
+			     snode_idx);
+	      delete *out_problem;
+	      *out_problem = NULL;
+	      return best_concat_epath;
+	    }
+	  else
+	    {
+	      if (logger)
+		logger->log ("rejecting %qs at EN: %i, SN: %i"
+			     " due to infeasible path",
+			     desc, enode->m_index,
+			     snode_idx);
+	      return NULL;
+	    }
+	}
+      else
+	{
+	  if (logger)
+	    logger->log ("accepting %qs at EN: %i, SN: %i"
+			 " despite infeasible path (due to %qs)",
+			 desc, enode->m_index,
+			 snode_idx,
+			 "-fno-analyzer-feasibility");
+	}
+    }
+
+  return epath;
+}
+
+/* FIXME.  */
+
+static bool
+interesting_p (const exploded_node *enode)
+{
+  if (enode->m_preds.length () > 1)
+    return true;
+  if (enode->m_preds.length () == 1)
+    {
+      const exploded_node *pred = enode->m_preds[0]->m_src;
+      if (pred->m_succs.length () > 1)
+	return true;
+    }
+  return false;
+}
+
+/* FIXME.  */
+
+exploded_path *
+epath_finder::get_best_concat_epath (const exploded_node *enode)
+{
+  logger *logger = get_logger ();
+  LOG_SCOPE (logger);
+  /* TODO: Try harder; might need to clobber/delete *out_problem.
+     Might want a cleaner interface for this.  */
+
+  /* Create trimmed graph for just ENODE.
+     Iterate through modifications.
+       Create mutated graph expressing modifications.
+       Find shortest path through modification.
+       Is it feasible?
+       Consider it.
+     Return the shortest feasible path that was found
+     via modifications.
+
+     But we'll also want to debug feasibility issues, I guess.
+     Modifications: perhaps require visiting an enode first,
+     for "interesting" enodes, where enodes are
+     (i) ones that weren't in the epath we found earlier
+     (ii) ones whose predecessor has more than one out-edge
+     (i.e. there was a choice beforehand)
+     Does that get us further?
+     Would be interesting to list those enodes, when logging.
+     (and maybe highlight them in a custom dump?)
+     Do we actually have to build a trimmed graph, or merely build
+     the subset of interest?
+     (and trimming could be confusing when dealing with indexes).  */
+  // FIXME: find "interesting" enodes:
+  unsigned i;
+  exploded_node *iter_enode;
+  exploded_path *best_concat_epath = NULL;
+  exploded_node *best_iter_enode = NULL;
+  FOR_EACH_VEC_ELT (m_eg.m_nodes, i, iter_enode)
+    {
+      if (iter_enode == enode)
+	continue;
+      if (interesting_p (iter_enode))
+	{
+	  if (logger)
+	    logger->log ("EN: %i is interesting", iter_enode->m_index);
+	  // TODO
+	  // FIXME: also, this set is constant per EG; cache all of this
+	  /* Find the ones for which there's a feasible shortest path,
+	     and use these a prefixes.  */
+	  // FIXME: should we recurse here?
+	  exploded_path *prefix_epath
+	    = new exploded_path (m_sep.get_shortest_path (iter_enode));
+	  if (prefix_epath->feasible_p (logger, NULL,
+					m_eg.get_engine (), &m_eg))
 	    {
-	      if (iter_enode == enode)
-		continue;
-	      if (interesting_p (iter_enode))
+	      if (logger)
+		logger->log ("prefix path to EN: %i is feasible",
+			     iter_enode->m_index);
+	      /* Find shortest path from iter_enode to ENODE.  */
+	      shortest_exploded_paths sep_from_prefix (m_eg, iter_enode);
+	      exploded_path suffix_epath
+		(sep_from_prefix.get_shortest_path (enode));
+
+	      /* Bail if no path exists.  */
+	      if (suffix_epath.length () == 0)
 		{
 		  if (logger)
-		    logger->log ("EN: %i is interesting", iter_enode->m_index);
-		  // TODO
-		  // FIXME: also, this set is constant per EG; cache all of this
-		  /* Find the ones for which there's a feasible shortest path,
-		     and use these a prefixes.  */
-		  // FIXME: should we recurse here?
-		  exploded_path *prefix_epath
-		    = new exploded_path (m_sep.get_shortest_path (iter_enode));
-		  if (prefix_epath->feasible_p (logger, NULL,
-						m_eg.get_engine (), &m_eg))
-		    {
-		      if (logger)
-			logger->log ("prefix path to EN: %i is feasible",
-				     iter_enode->m_index);
-		      /* Find shortest path from iter_enode to ENODE.  */
-		      shortest_exploded_paths sep_from_prefix (m_eg, iter_enode);
-		      exploded_path suffix_epath
-			(sep_from_prefix.get_shortest_path (enode));
-
-		      /* Bail if no path exists.  */
-		      if (suffix_epath.length () == 0)
-			{
-			  if (logger)
-			    logger->log ("no suffix path from EN: %i to EN: %i",
-					 iter_enode->m_index, enode->m_index);
-			  continue;
-			}
+		    logger->log ("no suffix path from EN: %i to EN: %i",
+				 iter_enode->m_index, enode->m_index);
+		  continue;
+		}
 
-		      /* Concate prefix and suffix.  */
-		      exploded_path *concat_epath
-			= new exploded_path (*prefix_epath, suffix_epath);
-		      if (concat_epath->feasible_p (logger, NULL,
-						    m_eg.get_engine (), &m_eg))
+	      /* Concate prefix and suffix.  */
+	      exploded_path *concat_epath
+		= new exploded_path (*prefix_epath, suffix_epath);
+	      if (concat_epath->feasible_p (logger, NULL,
+					    m_eg.get_engine (), &m_eg))
+		{
+		  if (logger)
+		    logger->log ("concat path via EN: %i is feasible",
+				 iter_enode->m_index);
+		  if (best_concat_epath)
+		    {
+		      if (concat_epath->length ()
+			  < best_concat_epath->length ())
 			{
 			  if (logger)
-			    logger->log ("concat path via EN: %i is feasible",
-					 iter_enode->m_index);
-			  if (best_concat_epath)
-			    {
-			      if (concat_epath->length ()
-				  < best_concat_epath->length ())
-				{
-				  if (logger)
-				    logger->log
-				      ("length (%i) beats existing best (%i)",
-				       concat_epath->length (),
-				       best_concat_epath->length ());
-				  delete best_concat_epath;
-				  best_concat_epath = concat_epath;
-				  best_iter_enode = iter_enode;
-				}
-			      else
-				{
-				  if (logger)
-				    logger->log
-				      ("length (%i) does not beat"
-				       " existing best (%i)",
-				       concat_epath->length (),
-				       best_concat_epath->length ());
-				  delete concat_epath;
-				}
-			    }
-			  else
-			    {
-			      if (logger)
-				logger->log
-				  ("first feasible concat path (length: %i)",
-				   concat_epath->length ());
-			      best_concat_epath = concat_epath;
-			      best_iter_enode = iter_enode;
-			    }
+			    logger->log
+			      ("length (%i) beats existing best (%i)",
+			       concat_epath->length (),
+			       best_concat_epath->length ());
+			  delete best_concat_epath;
+			  best_concat_epath = concat_epath;
+			  best_iter_enode = iter_enode;
 			}
 		      else
 			{
 			  if (logger)
 			    logger->log
-			      ("concat path via EN: %i is not feasible",
-			       iter_enode->m_index);
+			      ("length (%i) does not beat"
+			       " existing best (%i)",
+			       concat_epath->length (),
+			       best_concat_epath->length ());
 			  delete concat_epath;
 			}
-
-		      /* Concat them; is it feasible?
-			 TODO: reuse the state for the endpoint of the
-			 prefix path. */
-		      // TODO
 		    }
 		  else
 		    {
 		      if (logger)
-			logger->log ("prefix path to EN: %i is not feasible",
-				     iter_enode->m_index);
+			logger->log
+			  ("first feasible concat path (length: %i)",
+			   concat_epath->length ());
+		      best_concat_epath = concat_epath;
+		      best_iter_enode = iter_enode;
 		    }
-		  delete prefix_epath;
 		}
-	    }
+	      else
+		{
+		  if (logger)
+		    logger->log
+		      ("concat path via EN: %i is not feasible",
+		       iter_enode->m_index);
+		  delete concat_epath;
+		}
 
-	  if (best_concat_epath)
-	    {
-	      if (logger)
-		logger->log ("found a feasible concat epath to EN: %i"
-			     " via EN: %i",
-			     enode->m_index,
-			     best_iter_enode->m_index);
-	      delete *out_problem;
-	      *out_problem = NULL;
-	      return best_concat_epath;
+	      /* Concat them; is it feasible?
+		 TODO: reuse the state for the endpoint of the
+		 prefix path. */
+	      // TODO
 	    }
 	  else
 	    {
 	      if (logger)
-		logger->log ("did not find a feasible concat epath to EN: %i",
-			     enode->m_index);
-	      return NULL;
+		logger->log ("prefix path to EN: %i is not feasible",
+			     iter_enode->m_index);
 	    }
-	}
-      else
-	{
-	  if (logger)
-	    logger->log ("accepting %qs at EN: %i, SN: %i"
-			 " despite infeasible path (due to %qs)",
-			 desc, enode->m_index,
-			 snode_idx,
-			 "-fno-analyzer-feasibility");
+	  delete prefix_epath;
 	}
     }
 
-  return epath;
+  if (best_concat_epath)
+    {
+      if (logger)
+	logger->log ("found a feasible concat epath to EN: %i"
+		     " (best is via EN: %i)",
+		     enode->m_index,
+		     best_iter_enode->m_index);
+      return best_concat_epath;
+    }
+  else
+    {
+      if (logger)
+	logger->log ("did not find a feasible concat epath to EN: %i",
+		     enode->m_index);
+      return NULL;
+    }
 }
 
 /* class saved_diagnostic.  */
-- 
2.26.2

