Re: A couple of changes to Soot

[Navindra Umanee <navindra@cs.mcgill.ca>]

Hi Venkatesh,

Venkatesh Prasad Ranganath <vranganath@cox.net> wrote:
> I have made some simple changes to Soot and would like to release it
> into Soot.  The changes are to enable SimpleLocalXXX to work on
> UnitGraphs rather than CompleteUnitGraphs.  This stems from the fact

Thank you for your contribution!  

These changes have been committed to soot/trunk in revision 1286 and
should be present in the next Soot release.  One of your submitted
changes was however not applied (read on).  
--------

I believe the original requirement for CompleteUnitGraph was out of
concern for correctness of the analysis.  For example, consider
a SimpleLocalDefs analysis on:

    public static void deftest()
    {
        int d = 0;

        try{
            d = 1;
        }
        catch(Exception e){
            int c = d;
        }
    }

If you provide SimpleLocalDefs with a BriefUnitGraph instead of a
CompleteUnitGraph as your change would allow, the analysis will claim
that there are no reaching defs of d at "int c = d" -- this is
obviously wrong.

On the other hand, a user may have a specially-tailored implementation
of UnitGraph (eg, John's pruned exception graph) and may wish to pass
it to SimpleLocalDefs...  so it is indeed desireable to retain the
genericity.  Furthermore, I agree with your claim that is up to the
user to decide which UnitGraph to use and to decide for him/herself
whether something such as BriefUnitGraph would be suitable or not.
--------

This part of your patch was not applied:

--- build.xml   (revision 1280)
+++ build.xml   (working copy)
@@ -64,6 +64,7 @@
             destdir="doc"
             maxmemory="200m"
             windowtitle="Soot API"
+            private="yes"
         >
             <fileset dir="src" includes="**/*.java"/>
         </javadoc>

Was this submission intentional?  If so, could you elaborate why you
think it might be desireable to include information for private
classes and members in the official Soot API documentation?
--------

As an aside, although the supplied diff was in the correct format (and
it is awesome to see you using the SVN Soot repository), the patch
contained many whitespace modifications for a few lines of changes.
If it is not possible to avoid whitespace changes in the future, could
you try using something such as:

svn diff --diff-cmd '/usr/bin/diff' -x '-b -w' 

when creating your patch?  This would help greatly when reviewing!

(Note: If you are using Windows, I'm afraid I'm not sure what the
equivalent diff command is.)  
--------

Thanks again for your contribution!  Hope to see more in the future!

Cheers,
Navin.