I realized that this check is also brittle
assert(!departEdge || departEdge->target());

Both inEdgeFromAny and outEdgeToAny are not NULLPartEdge. It should be something like
assert( (departEdge? departEdge->target() : false) || !departEdge)
I’m pretty sure these are identical. They’re just computing “if (departEdge!=NULL) then (departEdge->target()!=NULL)”. In other words, if it’s a non-null edge, it must be fully concrete or inEdgeFromAny(). (same thing for getLatticeBelow for outEdgeToAny())


if(!departEdge || !departEdge->source()) departEdge = NULLPartEdge;
should be
if(!departEdge && !departEdge->source()) departEdge = NULLPartEdge;
This version will crash because you’ll check departEdge->source() when departEdge is NULL. I think the right code is
if(departEdge && !departEdge->source()) departEdge = NULLPartEdge;


Regarding checking for invalidity of concrete edge, consider this: given a concrete edge to this function and if I have info on NULLPartEdge, getLattice_ex should not abort right ? We are counting on getLattice_ex to return NULL when we don't find the info for the given edge.
Yeah, you should check for the case where the only key is NULLPartEdge in getLattice_ex().

Greg Bronevetsky
Lawrence Livermore National Lab
(925) 424-5756
bronevetsky@llnl.gov<mailto:bronevetsky@llnl.gov>
http://greg.bronevetsky.com

From: Sriram Aananthakrishnan [mailto:notifications@github.com]
Sent: Wednesday, November 13, 2013 2:54 PM
To: sriram87/edg4x-rose
Cc: bronevet
Subject: Re: [edg4x-rose] Fuse state access (#2)


In projects/fuse/src/state/nodeState.C:

> +  Lattice* retLattice;

> +  scope reg("NodeState::getLatticeAbove", scope::medium, attrGE("nodeStateDebugLevel", 2));

> +  if(nodeStateDebugLevel() >= 2) {

> +    dbg << "analysis=" << dynamic_cast<ComposedAnalysis*>(analysis)->str()

> +        << ", PartEdge=" << (departEdge? departEdge->str() : "NULL")

> +        << ", latticeName=" << latticeName << endl;

> +  }

> +

> +  // if the departEdge is not NULLPartEdge

> +  // assert that we always get specific edge or inEdgeFromAny

> +  // if the info is not available query for info on NULLPartEdge

> +  if(departEdge) {

> +    assert(departEdge->target());

> +    retLattice = getLattice_ex(dfInfoAbove, analysis, departEdge, latticeName);

> +    if(!retLattice) {

> +      retLattice = getLatticeAbove(analysis, NULLPartEdge, latticeName);

I realized that this check is also brittle
assert(!departEdge || departEdge->target());

Both inEdgeFromAny and outEdgeToAny are not NULLPartEdge. It should be something like
assert( (departEdge? departEdge->target() : false) || !departEdge)

if(!departEdge || !departEdge->source()) departEdge = NULLPartEdge;
should be
if(!departEdge && !departEdge->source()) departEdge = NULLPartEdge;

Regarding checking for invalidity of concrete edge, consider this: given a concrete edge to this function and if I have info on NULLPartEdge, getLattice_ex should not abort right ? We are counting on getLattice_ex to return NULL when we don't find the info for the given edge.

—
Reply to this email directly or view it on GitHub<https://github.com/sriram87/edg4x-rose/pull/2/files#r7644348>.