Comment 1 Marcel Bruch 2011-02-20 06:46:34 EST

Hi Sebastian,

the definition site handling is a bit clumsy at the moment. Maybe we can work out  a solution here?

We have four kinds of ObjectInstanceKeys we would like to distinguish:

# FIELD
# PARAMETER
# LOCAL
# THIS

A FIELD is always defined by some field declaration. THIS is also pretty clear. The distinction between LOCAL and PARAMETER, however, is somewhat artificial because the both are locals in some sense. The only difference between a local and a parameter is how they are defined. A PARAMETER is always defined by the entry point of the analysis (i.e., the method we start the analysis). A LOCAL is either defined by a 

# NEW, or 
# METHOD_RETURN, or 
# PHI (some path in the static execution which may point to many different values if statements, loops etc.) We refer to this as UNKNOWN.

Furthermore, each ObjectInstanceKey my be refined by a cast (which is currently not considered at all).

Given these kinds of how values can be defined, we get the following definition sites:

for fields:
	the field reference containing the name, the type and the declaring class

for this:
	the declared class type or maybe the super type and its implemented interfaces since this is what we need for analysis

for parameters:
	the method name, the argument index, the argument type (implicitly defined by the argument index and method name already)

for locals-new:
	the constructor call site?

for locals-method-return:
	the call site that returned this local? Something like source: myMethod(), type:IWorkbench, defined-by: PlatformUI.getWorkbench()?

for phis:
	UNKNOWN?


This is how the updated definition site could look like:

public class DefinitionSite2 {

    public enum Kind {
        THIS, FIELD, PARAMETER, METHOD_RETURN, NEW, UNKNOWN
    }

    /**
     * @param field
     *            something like MyClass.fieldName:LString;
     */
    public DefinitionSite2 forField(final IFieldName field) {
        final DefinitionSite2 res = new DefinitionSite2();
        res.kind = Kind.FIELD;
        res.definedByField = field;
        return res;
    }

    /**
     * @param analysisEntrypointMethod
     *            something like MyDialogPage.createContents(LComposite;)V
     * @param parameterIndex
     *            0 for LComposite in above example
     */
    public DefinitionSite2 forParameter(final IMethodName analysisEntrypointMethod, final int parameterIndex) {
        final DefinitionSite2 res = new DefinitionSite2();
        res.kind = Kind.PARAMETER;
        res.definedByMethod = analysisEntrypointMethod;
        res.parameterIndex = parameterIndex;
        return res;
    }

    /**
     * @param definingMethod
     *            something like "PlatformUI.getWorkbench()LWorkbench;"
     */
    public DefinitionSite2 forMethodReturn(final IMethodName definingMethod) {
        final DefinitionSite2 res = new DefinitionSite2();
        res.kind = Kind.METHOD_RETURN;
        res.definedByMethod = definingMethod;
        return res;
    }

    public DefinitionSite2 forNew() {
        final DefinitionSite2 res = new DefinitionSite2();
        res.kind = Kind.NEW;
        // no definedByMethod. Constructor call can be identified quickly by
        // looking on the receiver call sites.
        return res;
    }

    public DefinitionSite2 forThis() {
        final DefinitionSite2 res = new DefinitionSite2();
        res.kind = Kind.THIS;
        // no definedByMethod. Constructor call can be identified quickly by
        // looking on the receiver call sites.
        return res;
    }

    public DefinitionSite2 forUnknown() {
        final DefinitionSite2 res = new DefinitionSite2();
        res.kind = Kind.UNKNOWN;
        return res;
    }

    public Kind kind;
    public IFieldName definedByField;
    public IMethodName definedByMethod;
    public int parameterIndex;

    @Override
    public String toString() {
        return ToStringBuilder.reflectionToString(this, ToStringStyle.MULTI_LINE_STYLE);
    }
}

Suggestions?

Comment 2 Sebastian Proksch 2011-02-21 09:39:20 EST

I think this will work (under the assumption, that null values for the definition site are history then ;)).

My only suggestion would be a slightly different handling for "new": A shortcoming of the actual implementation is in my opinion, that <init> is a special case, that has to be handled a lot (everytime you working with reciever call sites you have to check whether there is an <init> and take care of it if necessary).

I would prefer a (single) special handling for <init> while analyzing, that removes the constructor call from the callsites, but records it as a "definedByMethod"... I think of a constructor call more as a property of the definition site "NEW", rather than a method call for the object...

What do you think about the following approach:

(In reply to comment #1)
> ...
>     public DefinitionSite2 forNew(final IMethodName constructorCall) {
>         final DefinitionSite2 res = new DefinitionSite2();
>         res.kind = Kind.NEW;
>         res.definedByMethdod = constructorCall;
>         return res;
>     }
> ...

If you need to know if/which constructor was called, its easy to reconstruct by inspecting the definition site, if you don't need it, everything is fine.

Do I miss something here?

Comment 3 Sebastian Proksch 2011-02-21 09:53:09 EST

After some reflection of my own post... doesn't it make sense to keep NEW and METHOD_RETURN merged as LOCAL and just assign a "definedByMethod" for both?

Do we really need a distinction based on "Kind" between method returns and <init>, if we record it the way I just proposed?

Comment 4 Marcel Bruch 2012-02-02 01:57:52 EST

Do you have time for data structure discussion today?

Comment 5 Marcel Bruch 2012-04-26 08:11:14 EDT

I guess this has been adressed lately, right? Closing.

Comment 6 Marcel Bruch 2012-06-09 15:12:00 EDT

Set target milestone for fixed bugs to 1.0

Comment 7 Marcel Bruch 2012-06-09 15:12:14 EDT

Set target milestone for fixed bugs to 1.0