Bug 439516

Comment 1 Stephan Herrmann 2014-07-14 06:05:51 EDT

Mike, I believe this is a weakness in the CLIMB-to-top rule: (to me) it is unclear if there is/should be a difference between "C<T>" and "C<T extends Object>".

My understanding is that a difference is intended, but technically that's not possible (see comment 0).

How does the Checkers framework handle this?

Comment 2 Michael Ernst 2014-07-19 09:32:15 EDT

Stephan-

You are correct that 
   C1<T>
and
   C2<T extends Object>
produce identical bytecode.  The Checker Framework can distinguish them because it is a source code tool.  A bytecode tool is more limited and cannot distinguish them.

The distinction is intentional:  it's not a weakness in the CLIMB-to-top rule, but something we added because it improves usability.

As described at
http://types.cs.washington.edu/checker-framework/current/checker-framework-manual.html#climb-to-top ,  the rationale is that 
   C1<T>
means "no restriction at all on the instantiation of T", whereas
   C2<T extends D>
means "some restriction on the instantiation of T", even if D is Object.  Furthermore, it would be weird for the "Object" in
   C2<T extends Object>
to mean something different than "Object" at nearby places in the code, such as in List<Object>.

This rule means that the upper bound of
  class C<T>
is defaulted differently than the upper bound
  class C<T extends Object>
.  It would be more confusing for "Object" in
  class C<T extends Object>
to be defaulted differently than "Object" in an instantiation
  C<Object>
and for
  class C<T extends Object>
to
be defaulted differently than
  class C<T extends Date>

I agree that it is a reasonable pragmatic choice for a bytecode tool to always consider upper bounds of j.l.Object as "implicit" bounds.  That is what I would do, faced with the need to implement a bytecode tool that has to work on bytecodes produced by a compiler that does not support pluggable types.

At the source code level, I still prefer the CLIMB-to-top choice.  A source code tool such as the Checker Framework or Eclipse can output explicit annotations on every upper bound, so there is no ambiguity for the bytecode tool and it doesn't have to do any defaulting or inference.

Comment 3 Stephan Herrmann 2014-07-19 12:56:24 EDT

(In reply to Michael Ernst from comment #2)

Thanks, Mike, for weighing in.

Unfortunately, I'm bound to treating ecj as a bytecode tool, not only for compatibility with other compilers but also to support incremental compilation.

> At the source code level, I still prefer the CLIMB-to-top choice.  A source
> code tool such as the Checker Framework or Eclipse can output explicit
> annotations on every upper bound, so there is no ambiguity for the bytecode
> tool and it doesn't have to do any defaulting or inference.

We had a similar discussion in bug 366063 and my strategy of generating synthetic annotations has been voted down. Hence ecj cannot distinguish C<T> from C<T extends Object>.

> I agree that it is a reasonable pragmatic choice for a bytecode tool to
> always consider upper bounds of j.l.Object as "implicit" bounds.  That is
> what I would do, faced with the need to implement a bytecode tool that has
> to work on bytecodes produced by a compiler that does not support pluggable
> types.

In our case, even consuming bytecode from our own compiler doesn't help.

Cc'ing Markus as the driver of bug 366063. If that decision still holds, I'll have to make a compromise along the following lines:

Within the scope of @NonNullByDefault:

- C<T> means: C<T>
- C<T extends Foo> means: C<T extends @NonNull Foo>
- C<T extends Object> means: C<T extends Object>

The third case is weird and deserves a compiler warning.
We may consider this weirdness to be tolerable, though, because it shouldn't occur normally in real life code.
For code that avoids this warning, the semantics of ecj and checkers framework are the same.

Comment 4 Stephan Herrmann 2014-07-22 11:55:00 EDT

Resolved for 4.5 M1 by these commits:

http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=63231f253dc3aaae18caf57a7f77da85f8cefe96
Don't apply nonnull-default on <T extends Object> when read from .class.


http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=eba39b7d16632d43ea13707b696b73d1b911dc13
For source level type parameters of the shape <T extends Object> do not apply the nonnull default to the type bound.
Instead raise a compiler warning saying that the default for explicit type bounds does not apply here. 
Also mention this fact in the javadoc of DefaultLocation.TYPE_BOUND

This solution is not perfect, but within the given constraints a perfect solution doesn't seem to be possible.

Comment 5 Srikanth Sankaran 2014-08-06 02:08:58 EDT

Verified for 4.5 M1 using code inspection.

Comment 6 Stephan Herrmann 2015-01-15 07:32:51 EST

(In reply to Stephan Herrmann from comment #4)
> http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/
> ?id=63231f253dc3aaae18caf57a7f77da85f8cefe96
> Don't apply nonnull-default on <T extends Object> when read from .class.

This part has been back ported for 4.4.2 as prerequisite for bug 453475 (commit e0b75d21a26e97c9a42f6cf8d270088b82d36613).