[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: A couple of changes to Soot
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.