DOC-56: An overview of grapht and dependence_grapht in corresponding README.md. #4095

marek-trtik · 2019-02-05T17:06:46Z

Besides an overview of dependence_graph I've also added a brief (and incomplete) overview of the grapht class, because it might be useful to understand how to use a dependence graph.

hannes-steffenhagen-diffblue · 2019-02-05T17:38:47Z

src/analyses/README.md

+### Dependence graph
+
+Implemented in `src/analyses/dependence_graph.h(cpp)`. It is a graph and an
+abstract interpreter in the same time. The abstract interpretation nature


at the same time

nature of abstract interpretation?

The sentence says "nature of the dependence graph being (also) AI analyser" rather than "nature of abstract interpretation".

hannes-steffenhagen-diffblue · 2019-02-05T17:39:46Z

src/analyses/README.md

+
+Implemented in `src/analyses/dependence_graph.h(cpp)`. It is a graph and an
+abstract interpreter in the same time. The abstract interpretation nature
+allows a dependence graph to build itself (the graph) from a given GOTO program.


what does it mean for a graph to build itself?

For a graph in general it would perhaps mean nothing. However, here we speak about a special graph - the dependence graph in our code base. It is not just a graph representing control/data-dependencies is some GOTO program. It is also AI analyser. You do not build it by calling its graph-interface methods for adding nodes and edges. You instead pass it GOTO program and it will run its AI-interface methods on it to perform the data- and control- dependency analyses (i.e. it uses its "AI" nature of being also an AI analyser) and then it runs its finalize method to actually build the graph by calling graph-interface methods (to add control/data-dependency edges) from the results of the analyses.

I think the wording here ("build itself") is just a bit weird, below you very nicely explain what is going on and it may be worth having a forward reference to that.

hannes-steffenhagen-diffblue · 2019-02-05T17:40:53Z

src/analyses/README.md

+A dependence graph extends the class `grapht` with `dep_nodet` as the type of
+nodes (see `src/util/graph.h` for more details about
+[graph representation](../util/README.md)).
+The `dep_nodet` extends `graph_nodet<dep_edget>` by an iterator to a GOTO


with an iterator?

hannes-steffenhagen-diffblue · 2019-02-05T17:41:22Z

src/analyses/README.md

+nodes (see `src/util/graph.h` for more details about
+[graph representation](../util/README.md)).
+The `dep_nodet` extends `graph_nodet<dep_edget>` by an iterator to a GOTO
+program instruction. It means that each graph node corresponds to a particular


what is "It" here?

The information of the previous sentence - the fact that each node stores an iterator to a GOTO instruction.

hannes-steffenhagen-diffblue · 2019-02-05T17:43:12Z

src/analyses/README.md

+program instruction. It means that each graph node corresponds to a particular
+program instruction. A labelled edge `(u, v)` of a dependence graph expresses
+a dependency of the program instruction corresponding to node `u` on the program
+instruction corresponding to node `v`. The label of the edge (i.e. data of the


writing style guide recommends no latin (i.e.)

hannes-steffenhagen-diffblue · 2019-02-05T17:51:04Z

src/util/README.md

+Implemented in `src/util/graph.h` as `grapht` class. The class natively defines
+only a directional graph. However, an undirected graph can be emulated by
+inserting for each edge (u, v) also (v, u). A multi-graph is not supported
+though, because parallel edges are not allowed between vertices. 


I believe this can be simplified to "The grapht class represents a directed graph"; I don't think there's normally an expectation that any graph representation can also represent a multi graph directly, and a multi graph can be embedded in a directed graph in a similar way to how undirected graphs can be represented, so I don't necessarily think it's worth calling this out as a deficiency.

hannes-steffenhagen-diffblue · 2019-02-05T17:53:43Z

src/util/README.md

+
+#### Data representation
+
+A graph is defined by a template class `grapht<N>`, where the type `N` defines


where N is the type of the nodes of the graph?

hannes-steffenhagen-diffblue · 2019-02-05T17:57:19Z

src/util/README.md

+- It must be default constructible; the common way how to insert a node to the
+  graph is to call method `add_node` which pushes a new default-constructed
+  instance of `N` to the end of the vector and returns its index.
+- It must define a type `edget`, representing type of data attached to any edge


I wonder why we do it this way; I can't see any obvious reason why the edge type necessarily follows from the node type. It seems the type parameter here doesn't specify the node type, but actually both the node type and the edge type? Maybe it would be worth refactoring the graph to work like this grapht<graph_configt> { using node_t = typename graph_configt::node_t; using edge_t = graph_configt::edge_t; // ... }? Just a thought

Indeed we just define edges and node types at the same time. But of course this can be improved upon, and improvements are always welcome :-)

hannes-steffenhagen-diffblue · 2019-02-05T17:58:55Z

src/util/README.md

+  of that node, then `(v, u)` (resp. `(u, v)`) represent an edge in the graph,
+  with attached data `in[v]` (resp. `out[v]`).
+- It must define methods `add_in`, `erase_in`, and `add_out`, `erase_out` for
+  insertion and removal of values to and from the maps `in` and `out`.


I wonder why we bother having this stuff on the node type at all if all nodes are required to implement them in the same way anyway

hannes-steffenhagen-diffblue · 2019-02-05T17:59:18Z

src/util/README.md

+  insertion and removal of values to and from the maps `in` and `out`.
+- It must define a method `pretty` converting the node to a "pretty" string. 
+
+One can use a predefined template class `graph_nodet<E>` as the template


What is E here?

marek-trtik · 2019-02-06T11:48:54Z

Requested updates are available.

tautschnig · 2019-02-06T15:32:22Z

src/analyses/README.md

+
+Implemented in `src/analyses/dependence_graph.h(cpp)`. It is a graph and an
+abstract interpreter in the same time. The abstract interpretation nature
+allows a dependence graph to build itself (the graph) from a given GOTO program.


I think the wording here ("build itself") is just a bit weird, below you very nicely explain what is going on and it may be worth having a forward reference to that.

marek-trtik · 2019-02-07T10:45:48Z

Updates requested in the review are available: Added forward link from "build itself" to the Construction sub-section.

tautschnig · 2019-02-08T10:27:19Z

@marek-trtik This requires a rebase to pick up the Windows CI fix. And doxygen says: README.md:105: warning: unexpected token TK_EOF as the argument of ref.

marek-trtik · 2019-02-11T12:15:51Z

Thanks Michael! Issue fixed, commits squashed, and rebased to latest develop.

allredj

✔️
Passed Diffblue compatibility checks (cbmc commit: a17a64a).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/100445165

marek-trtik added the documentation label Feb 5, 2019

marek-trtik self-assigned this Feb 5, 2019

marek-trtik requested review from chrisr-diffblue, kroening, martin-cs, peterschrammel, pkesseli, smowton, tautschnig and thk123 as code owners February 5, 2019 17:06

marek-trtik force-pushed the doc_dep_graph branch from dde98e0 to 1da964c Compare February 5, 2019 17:40

hannes-steffenhagen-diffblue reviewed Feb 5, 2019

View reviewed changes

tautschnig approved these changes Feb 6, 2019

View reviewed changes

tautschnig assigned chrisr-diffblue and hannes-steffenhagen-diffblue Feb 6, 2019

An overview of grapht and dependence_grapht in corresponding README.md.

a17a64a

marek-trtik force-pushed the doc_dep_graph branch from 85524de to a17a64a Compare February 11, 2019 12:14

marek-trtik requested a review from danpoe as a code owner February 11, 2019 12:14

tautschnig assigned peterschrammel and unassigned marek-trtik and hannes-steffenhagen-diffblue Feb 11, 2019

allredj reviewed Feb 11, 2019

View reviewed changes

peterschrammel approved these changes Mar 6, 2019

View reviewed changes

peterschrammel assigned marek-trtik and unassigned peterschrammel Mar 6, 2019

tautschnig merged commit 107d5a8 into diffblue:develop Mar 6, 2019


		#### Data representation

		A graph is defined by a template class `grapht<N>`, where the type `N` defines

DOC-56: An overview of grapht and dependence_grapht in corresponding README.md. #4095

DOC-56: An overview of grapht and dependence_grapht in corresponding README.md. #4095

Uh oh!

Conversation

marek-trtik commented Feb 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marek-trtik Feb 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marek-trtik commented Feb 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marek-trtik commented Feb 7, 2019

Uh oh!

tautschnig commented Feb 8, 2019

Uh oh!

marek-trtik commented Feb 11, 2019

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marek-trtik Feb 6, 2019 •

edited

Loading