Re: A couple of changes to Soot

[Venkatesh Prasad Ranganath <vranganath@cox.net>]

Navindra Umanee wrote:
> 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!  

Thanks for accepting the changes.  I may have a few more changes (may be comparitively large and complex) due by the end of 
this fall and these are to enable Soot to support multiple instances of Scene (refer to my long mail on soot mailing list 
about the same topic for details) on a single JVM without touching JVM internals such as class loaders.  I would be interested 
to interact with anyone looking at the same issue as I know it is a touchy issue for the Soot maintainers.  I welcome any 
comments or suggestions.

> 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?

No. I just did svn diff and mailed it to you.  I guess this also explains the whitespace changes.  I will keep in mind your 
following suggestion.

> --------
>
> 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.


--

Venkatesh Prasad Ranganath,
Dept. Computing and Information Science,
Kansas State University, US.
web: http://www.cis.ksu.edu/~rvprasad