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.



> ///////
> +
> +/* 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?



> +/* 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?

+
> +/* 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 :-)

> +
> +class exploded_graph : public digraph<eg_traits>, public log_user
Multiple inheritance again?

Jeff

Reply via email to