About revision 1407.... Again

[John Jorgensen <jorgnsn@lcd.uregina.ca>]

>>>>> "vranganath" == Venkatesh Prasad Ranganath <vranganath@cox.net> writes:

    vranganath> Hi, In revision 1407, UnitGraph does not have a
    vranganath> UnitGraph(Body,boolean,boolean) constructor!!!
    vranganath> and this breaks my implementation :-(

Oops. The possibility that anybody would actually use the
UnitGraph constructors directly did not occur to me.  And it
should have, since I know of at least one other person who has
done so.

Before I go on to address your valid concern with backward
compatibility, let me explain my belief that UnitGraph should
have been abstract in the first place.

Although I am not the original author, from looking at the
UnitGraph code up to Soot 2.1.0, I feel confident that UnitGraph
was never meant to be used directly:  you are supposed to
instantiate BriefUnitGraph, or TrapUnitGraph, or
CompleteUnitGraph, rather than UnitGraph.  My evidence for that
is that the boolean flags to the UnitGraph constructor really
amount to switches between the subclasses:

  UnitGraph(body, false, false) == BriefUnitGraph(body)
  UnitGraph(body, false, true) == BriefUnitGraph(body)
  UnitGraph(body, true, false) == CompleteUnitGraph(body)
  UnitGraph(body, true, true) == TrapUnitGraph(body)

But if you instantiate a UnitGraph(body, true, false) in your
code, you effectively have a CompleteUnitGraph that doesn't know
it is a CompleteUnitGraph! I.e., any polymorphic methods of
CompleteUnitGraph will not be called on your method. Admittedly
that's a moot point in Soot 2.1.0, since there the subclasses of
UnitGraph do not add or override any methods of UnitGraph, but
they do with the arrival of ExceptionalUnitGraph.

UnitGraph's original author presumably found it easier to put the
common logic for constructing graphs into a common, parameterized
location, but he should---in my opinion---have made the UnitGraph
constructor protected to keep people from instantiating it the
way you have.  I'm trying to correct his mistake, but it looks
like I'm too late (there is a little more discussion of this in
section 3.2, "The New Graph Family Tree", of the technical report
at

  http://www.sable.mcgill.ca/publications/techreports/#report2003-3)

    vranganath> I understand the copy in the repository is
    vranganath> evolving.  However, if such a change will make
    vranganath> it's way into a subsequent release then I am
    vranganath> wondering how does the Soot team decide on such
    vranganath> API breaking changes?  I am not against changes,
    vranganath> but if it's a change affecting the published API
    vranganath> then shouldn't the user base be informed or
    vranganath> consulted before making such changes?

It just didn't occur to me that people would instantiate
UnitGraph directly, though as I admitted before, you are the
second person that I know of.  I still think that doing so is a
mistake that we should seek to prevent in future, because of how
it subverts the class hierarchy.  Maybe we can use you as a test
case of how big an imposition we would be forcing on our users.
Is it very difficult to replace your invocations of the UnitGraph
constructor with the corresponding constructor from my table of
equivalences, above, or do you depend on being able to mix
arbitrary values for the three parameters?

    vranganath> Can someone on the Soot team comment on the set
    vranganath> of changes exposed in revision 1407?  Is it wise
    vranganath> to develop against these changes assuming that
    vranganath> they will persist into the next release?

I'm no longer close enough to the core of Soot development to
comment on this.