On Wed, 2019-12-11 at 13:04 -0700, Jeff Law wrote: > On Fri, 2019-11-15 at 20:23 -0500, David Malcolm wrote: > > This patch adds exploded_graph and related classes, for managing > > exploring paths through the user's code as a directed graph > > of <point, state> pairs. > > > > gcc/ChangeLog: > > * analyzer/exploded-graph.h: New file. > > --- > > gcc/analyzer/exploded-graph.h | 754 > > ++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 754 insertions(+) > > create mode 100644 gcc/analyzer/exploded-graph.h > > > > diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded- > > graph.h > > new file mode 100644 > > index 0000000..f97d2b6 > > --- /dev/null > > +++ b/gcc/analyzer/exploded-graph.h > > @@ -0,0 +1,754 @@ > > +/* Classes for managing a directed graph of <point, state> pairs. > > + Copyright (C) 2019 Free Software Foundation, Inc. > > + Contributed by David Malcolm <dmalc...@redhat.com>. > > + > > +This file is part of GCC. > > + > > +GCC is free software; you can redistribute it and/or modify it > > +under the terms of the GNU General Public License as published by > > +the Free Software Foundation; either version 3, or (at your > > option) > > +any later version. > > + > > +GCC is distributed in the hope that it will be useful, but > > +WITHOUT ANY WARRANTY; without even the implied warranty of > > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > +General Public License for more details. > > + > > +You should have received a copy of the GNU General Public License > > +along with GCC; see the file COPYING3. If not see > > +<http://www.gnu.org/licenses/>;;. */ > > + > > +#ifndef GCC_ANALYZER_EXPLODED_GRAPH_H > > +#define GCC_ANALYZER_EXPLODED_GRAPH_H > > + > > +#include "fibonacci_heap.h" > > +#include "analyzer/analyzer-logging.h" > > +#include "analyzer/constraint-manager.h" > > +#include "analyzer/diagnostic-manager.h" > > +#include "analyzer/program-point.h" > > +#include "analyzer/program-state.h" > > +#include "analyzer/shortest-paths.h" > > + > > +////////////////////////////////////////////////////////////////// > > // > NIT. Is there some reason you don't just use whitespace for these > kind > of vertical separators. THe more I see them, the more they bug me, > probably because that's not been the style we've used for GCC.
I've been using them to highlight new "chapters" in a source file; typically there's another comment following them. I'm happy to drop the one where there isn't a trailing comment, but, I'm hoping there's an acceptable way to have an "section-header"-style comment. We have "^L" in a bunch of places, but my editor (emacs) doesn't do a great job of highlighting them. There's some precedence for: e.g. gengtype.c:5094:/******* Manage input files. ******/ lto-section-in.c has e.g.: /*****************************************************************************/ /* Record renamings of static declarations */ /*****************************************************************************/ tree-vect-loop-manip.c has: /************************************************************************* Simple Loop Peeling Utilities Utilities to support loop peeling for vectorization purposes. *************************************************************************/ but I suspect given how sporadic these are that this stuff snuck past review. Alternatively I can just drop these and retain the trailing comments, keeping their formatting. > > /////// > > + > > +/* Concrete implementation of region_model_context, wiring it up > > to > > the > > + rest of the analysis engine. */ > > + > > +class impl_region_model_context : public region_model_context, > > public log_user > Multiple inheritance? Is it really needed? Can we handle via > composition instead? It's handy here, but I can probably eliminate it. > > > +/* A <program_point, program_state> pair, used internally by > > + exploded_node as its immutable data, and as a key for > > identifying > > + exploded_nodes we've already seen in the graph. */ > > + > > +struct point_and_state > Shouldn't this be a class? Will fix (originally was just a pair of public fields, but then I added the cached hash value, so there's an argument for "class-ifying" this). > + > > +/* Per-program_point data for an exploded_graph. */ > > + > > +struct per_program_point_data > Here too? THere may be others. I'd suggest reviewing all your > "structs" and determine if we're better off calling them > "class". I'm > not going to insist on it though since I think the last discussion in > this space was to relax the conventions :-) Both fields are public, but they're not-POD. I'll look into them. > > + > > +class exploded_graph : public digraph<eg_traits>, public log_user > Multiple inheritance again? Again, it's handy here, but I think I can probably eliminate it. Dave
