Bug 325357

Comment 1 bungeman 2010-09-15 10:25:17 EDT

Created attachment 178935 [details]
Example.java

Comment 2 bungeman 2010-09-15 10:25:50 EDT

Created attachment 178936 [details]
Example2.java

Comment 3 bungeman 2010-09-15 10:26:14 EDT

Created attachment 178937 [details]
Example3.java

Comment 4 bungeman 2010-09-15 10:27:24 EDT

Changing to 3.6.0 where this was observed. I have not yet tested on anything newer.

Comment 5 bungeman 2010-09-15 10:33:31 EDT

I would like to point out that I'm leaving this at normal severity because there is a work-around, simply qualifying the types as coming from the base type works (as can be seen commented out in the examples). However, I do not believe this should be required (though it is probably best practice) because of the wording in JLS3. The inconsistency in member type inheritance seen here is the biggest issue.

Comment 6 Stephan Herrmann 2010-09-15 15:13:36 EDT

Stepping through super type resolution for Example.java I see an unfortunate
call chain (some intermediate functions omitted):

connectTypeHierarchy(Example)
- connectMemberTypes()
   - connectTypeHierarchy(Example$Leaf)
     - findSuperType(Base<A, InnerLeaf<A>>)
       - resolveTypeArgument(InnerLeaf<A>)
         - detectHierarchyCycle(InnerLeaf)
           - connectTypeHierarchyWithoutMembers(InnerLeaf)
             - findSuperType(InnerBase<B>)
               - getType(InnerBase)
                 - findMemberType(Leaf, InnerBase)

The last method finds that the hierarchy for Leaf is currently being
connected (started in frame 3) and aborts (returning null) .

It looks far from trivial to find a proper compilation order that
correctly resolves this kind of truly cyclic structure.

Comment 7 Stephan Herrmann 2010-09-15 16:14:45 EDT

A simplistic way of avoiding the cyclic checks would be to remove these lines
in ClassScope.detectHierarchyCycle(TypeBinding,TypeReference):

 // Reinstate the code deleted by the fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=205235
 // For details, see https://bugs.eclipse.org/bugs/show_bug.cgi?id=294057. 
 if ((superType.tagBits & TagBits.BeginHierarchyCheck) == 0 && superType instanceof SourceTypeBinding)
    // ensure if this is a source superclass that it has already been checked
    ((SourceTypeBinding) superType).scope.connectTypeHierarchyWithoutMembers();

Thus, we are in conflict with bug 294057, which was in conflict with 205235.
As expected, by removing said lines these two tests are broken:
  Java50Tests.testHierarchyNonCycle,
  Java50Tests.testHierarchyNonCycle2
But the issue of this bug *would* be fixed.

----

So much for the code analysis, what SHOULD be the right behavior?

Looking into bug 294057 it seems the error is reported intentionally,
as to align the behavior of javac and Eclipse. See this output from javac:

src/inherittest/Example.java:16: cannot find symbol
symbol  : class InnerExtendable
location: class inherittest.Example
        public static class Extendable<A extends ConcreteA, S extends InnerExtendable<A>> extends Base<A,S> {
                                                                      ^
src/inherittest/Example.java:20: cannot find symbol
symbol  : class InnerLeaf
location: class inherittest.Example
        public static class Leaf<A extends TypeA> extends Base<A, InnerLeaf<A>> {
                                                                  ^
2 errors

So javac and Eclipse now essentially agree, UNLESS you uncomment the 
import inherittest.Example.Base.InnerBase;
which makes Eclipse happy, while javac still isn't satisfied.
This makes the current bug slightly different from bug 294057.

Comment 8 Stephan Herrmann 2010-09-15 16:32:42 EDT

As an aside: to me all these examples look like very sad abuses of generics
to achieve something else: covariant inner classes. 
Of course standard Java doesn't have that.

In Object Teams you would simply write something like:

public class Example {
	
	public team class Base {
		public class TypeA { }
		public class Inner { }
	}
	
	public team class Extendable extends Base {
		@Override
		public class TypeA { }
		@Override
		public class Inner { }
	}
	
	public team class Leaf extends Base {
		@Override
		public class Inner { }
	}
}

And all the "complicated" typing just works.

Comment 9 bungeman 2010-09-15 17:06:45 EDT

I will agree that eclipse 3.6 does seem to be in agreement with javac1.6.0_18 (except for the oddity of uncommenting the <Z> in Example3.java, and uncommenting the import). However, because of the inconsistency between Raw* and Extendable working, and Leaf* not working, as well as the wording in the JLS3, I believe this behavior to be incorrect.

I am having some difficulty determining the exact the implications of the change in bug 294057, but it appears to have made eclipse 3.6 agree with javac1.6.0_18 which I am alleging is incorrect behavior. I believe the outcome of bug 294057 (and possibly this bug) should have been to correct JDT to match the JLS3 (it is unclear to me at this point if already was) and perhaps open a bug report against javac (and especially javac1.7.x, which appears to be failing greatly in the area, making code that compiles in javac1.6.x no longer compile).

So what I believe SHOULD be the right behavior is to follow the JLS3, which appears to have a nice consistent definition of what should happen here. I understand the pragmatic approach of following javac, but in this case I believe javac may be in error.

Given the varying outputs from different compilers, I believe at the very least these examples (or something like them) should be put into a test suite somewhere so that there is something to go by.

I was not previously aware of Object Teams, but I think you may be correct. The code these examples are simplified from is a generic visitor which can be instantiated and extended (and you can extend the return type of the visits). This would have benefited from a ThisType (without the ThisType this can be extended or instantiated, but not both, I got around this by forcing a 'safe' unsafe cast at one point). There is a lot of brain hurting generics going on in there, but as a result one cannot extend the visitor incorrectly without a compile error. I was actually surprised that this had to do with a member type not being inherited and not just something I got wrong (for real) in the generics.

Comment 10 Ayushman Jain 2010-09-16 04:16:28 EDT

Srikanth, please follow up. Thanks!

Comment 11 Stephan Herrmann 2010-09-16 08:30:00 EDT

Looking into the JLS I don't think the inheritance of member types
is at stake here. All compilers agree that the type InnerBase<A>
is available in the *body* of Leaf<A> (try by declaring a field
  InnerBase<A> field;
within Leaf<A>.

OTOH, as the compile errors vanish if you remove all type parameters,
nobody seems to question that InnerBase can be used as the superclass
of InnerLeaf.

The compiler reports an error only because it has detected a cyclic
dependency via the type parameters. So, what's the JLS have to say
about cyclic dependencies? I only found this in 8.1.4:
  "It is a compile-time error if a class depends on itself."
Unfortunately, the definition of "depends" pays no account to generics
nor to class nesting.

Srikanth, do you know of another JLS paragraph that justifies why this
particular kind of cyclic dependency is an error?

My feeling is that this is simply unspecified, and compiler implementers
have to extend the definition either
 - extend the notion of "depends" according to common sense
   -> may just want to change the error so it mentions cyclic dependency, or
 - do a best-effort resolving of types
   -> may want to *not* report the error, since all types *can* actually
      be resolved without violating any explicit rule of the JLS?

Unless we have evidence for or against a particular cycle-analysis,
it's probably still a good idea to basically stick to the javac behavior
and focus on the remaining differences, like, why do imports make
Eclipse happy, but not javac?

Comment 12 bungeman 2010-09-16 10:36:55 EDT

I am surprised that in Example2.java on the line

public static class InnerExtendable<A extends TypeA> extends Extendable.InnerBase<A> { }

I did not get a warning like 'The static type Extendable.InnerBase<A> should be accessed directly' similar to the warning one gets when accessing other static members in a non-static or indirect static context. A similar warning should even be given here when stating just 'InnerBase<A>' (which is implicitly the same) or practically anything other than 'Base.InnerBase<A>', since such indirection is probably always going to be confusing and not what was wanted. This may also be something to consider here while thinking about error messages. This situation is somewhat different from other static members, which are never inherited. It was very surprising to discover static type members are inherited.


The reason I believe the cyclic dependency clause should not apply (at the very least to static types used as generic type parameters, I'm not sure of the implications of inner types) is that the 'depends' section of the JLS3 states
-----
A class C directly depends on a type T if T is mentioned in the extends or implements clause of C either as a superclass or superinterface, or as a qualifier of a superclass or superinterface name. A class C depends on a reference type T if any of the following conditions hold:

    * C directly depends on T.
    * C directly depends on an interface I that depends (§9.1.3) on T.
    * C directly depends on a class D that depends on T (using this definition recursively). 

It is a compile-time error if a class depends on itself.
http://java.sun.com/docs/books/jls/third_edition/html/classes.html#271016
-----
While 'depends' is not defined here, I think that the bare 'depends' should be read as 'directly depends (recursively)', which makes the most sense. (Note that section 9.1.3 defines 'depends' for interfaces analogously.) The parts mentioned here are the bare classes, and so the generic type parameters of these classes should not count toward a class depending on itself. I believe that this does cover nesting with the 'qualifier' clause, and that it ignores generics because it was determined they should make no difference to the determination.

Comment 13 bungeman 2010-09-16 10:57:57 EDT

As additional evidence, I can't imagine a sane and consistent set of rules for explaining why, in Example3.java the following version of Leaf does not work

public static class Leaf extends Base<Leaf.InnerLeaf> {
    public static class InnerLeaf extends Leaf.InnerBase { }
}

but this does

public static class Leaf extends Base<Leaf.InnerLeaf> {
    public static class InnerLeaf extends Base.InnerBase { }
}

Since the actual InnerBase type being referenced in both cases is the same type. It may have different aliases (having been statically inherited) but it is still the same type. It is very odd, this member type inheritance, since it means that a single type can have multiple different fully qualified names. I believe that all such names should be fully substitutable since they are, in fact, all referring to the same type.

Comment 14 Stephan Herrmann 2014-11-20 05:45:41 EST

(In reply to bungeman from comment #13)
> As additional evidence, I can't imagine a sane and consistent set of rules
> for explaining why, in Example3.java the following version of Leaf does not
> work
> 
> public static class Leaf extends Base<Leaf.InnerLeaf> {
>     public static class InnerLeaf extends Leaf.InnerBase { }
> }
> 
> but this does
> 
> public static class Leaf extends Base<Leaf.InnerLeaf> {
>     public static class InnerLeaf extends Base.InnerBase { }
> }

I do see a critical difference: the name Base.InnerBase is a direct reference that points to an existing class. No other types touched for resolving this type reference. OTOH, Leaf.InnerBase can only be resolved after first resolving the superclass of Leaf, so the reference "Leaf.InnerBase" 'depends' on everything needed to create the type hierarchy of Leaf. The superclass of Leaf is not Base but "Base<Leaf.InnerLeaf>", which leads to the cycle.

To make the first example work, one would have to invent a concept of s.t. like "raw type hierarchy": resolve the type hierarchy of Leaf *disregarding* any type arguments. I don't think that this concept exists in JLS (but I admit that the JLS definition of "directly depends" seems to imply s.t. like this). 


What's our status here:
-----------------------

There is a simple workaround (acknowledged in comment 5): refer to static member types by their "real" qualified name, not via a subtype of an enclosing.

I don't think ecj is rejecting any programs that javac accepts, right?

From these I'm leaning towards closing as invalid.

If, OTOH, this issue arguably causes pain in real life, I'd first ask for clarification from the JLS authors to learn about their intention regarding this case.

-----

PS: *If* we decide to accept this example (even if javac doesn't), the following might work as a technical trick for breaking the cycle: given that an import statement seems to render the type reference resolvable, we could perhaps tweak a synthetic import once we see the extends clause. Since imports are not parameterized this *could* get us around the issue of resolving type arguments at an unhappy point in time. Not sure.

Comment 15 Eclipse Genie 2019-11-01 12:32:33 EDT

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.