Solution for access path problem

[pvojtechovsky]

After short googling, I have not found any java specification about access path yet (so I am really not a guru for that), anyway I understood that:

C1: Spoon needs access path from class A to super class B to be able to print the declaration of the class A correctly.

C2: There is no other place, where access path is needed

C3: Access path from class A to class B is not the qualified name of class B.

C4: the class B can have many access paths ~ one for each inheritance branch, but there is always only one qualified name of class B.

C5: the access path cannot be used to load the class using class loader. It can be done only using qualified name.

Please correct me, if I am wrong.

The solution I would like to discuss consists of S1 and S2:

S1: To remove the hack in JDTTreeBuilderHelper

It causes:

• the qualified name of the nested class is correct again, and can be used to load the class using class loader
• the relations nestedType.getDeclaringType() and declaringType.getNestedTypes() are consistent again

S2: To compute the access path from class A to class B during pretty printing

I am sure, that you get this idea too. Could you explain me what is the problem? And if computation of access path is problem, then could you show me the case where it is problem?

Thank You! And and have a nice dreams :-)

[monperrus]

S2: To compute the access path from class A to class B during pretty printing

I am sure, that you get this idea too.

No, not that I remember :-) It looks like an excellent idea!

[pvojtechovsky]

Is it good idea to have these methods in CtTypeReference API? They are actually used in ImportScannerImpl and DefaultJavaPrettyPrinter. What about names of these methods? Are they OK?

    /**
     * Checks visibility based on public, protected, package protected and private modifiers of type
     * @param type
     * @return true if this can access that type
     */
    boolean canAccess(CtTypeReference<?> type);

    /**
     * Returns this, or top level type of this, if this is an inner type
     */
    CtTypeReference<?> getTopLevelType();

What about name like getTopDeclaringType()?

    /**
     * Computes nearest access path parent from contextType to this type.
     *
     * Normally the declaring type can be used as access path. For example in this class hierarchy
     * <pre>
     * class A {
     *    class B {
     *       class C {}
     *    }
     * }
     * </pre>
     *
     * The C.getAccessParentFrom(null) will return B, because B can be used to access C, using code like <code>B.C</code><br>
     * But when some class (A or B) on the access path is not visible in type X, then we must found an alternative path.
     * For example in case like, when A and B are invisible, e.g because of modifier <code>protected</code>:
     * <pre>
     * class D extends B {
     * }
     * class X extends D {
     *   class F extends C
     * }
     * </pre>
     * The C.getAccessParentFrom(X) will return D, because D can be used to access C in scope of X.
     *
     * @param contextType - the type where the access path should be visible or null if we do not care about visibility
     * @return type reference which can be used to access this type in scope of contextType.
     */
    CtTypeReference<?> getAccessType(CtTypeReference<?> contextType);

[pvojtechovsky]

Please make a revision. I am finished with that... until you ask for some changes ;-)

[monperrus]

nice method.

[monperrus]

nice method.

[monperrus]

It looks good, thanks Pavel. But there is a significant performance cost. On MainTest only, what's the overhead?

Where is the performance bottleneck? Could you further optimize?

[pvojtechovsky]

performance

It must be a misundertanding. MainTest of this PR needs 33s. And MainTest of already merged PR needed 36s. So it is faster, not slower. I see it in Travis console output. Where do you see your numbers?

[pvojtechovsky]

In the commit 119bedb I did a performance optimization, which makes it 2 times faster on MS Windows (before MainTest needed 45s and now it taks 24s). I did it as part of this PR, by mistake... I had there infinite loop, which caused that MainTest failed on timeout in Travis. And before I found my infinite loop, I have made commit 119bedb

[pvojtechovsky]

I extracted the performance improvement to PR #990

[monperrus]

let's first converge on #990 and then rebase, measure and eventually
merge this one.

[pvojtechovsky]

rebased.

Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 34.334 sec - in spoon.test.main.MainTest

similar like others. I see no performance problem. But check it, I do not know what you have seen.

[tdurieux]

When we ask for the performance issue there was a huge execution time difference between your branch and the master.
Your branch probably didn't contain your optimization PRs.
@monperrus I think we can merge