Bug 470794 - Missing support for Intersection types in ITypeBinding
Summary: Missing support for Intersection types in ITypeBinding
Status: ASSIGNED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.4.2   Edit
Hardware: PC Windows 7
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-23 08:46 EDT by Missing name CLA
Modified: 2018-04-16 09:30 EDT (History)
3 users (show)

See Also:


Attachments
Proposed Patch wip (5.28 KB, patch)
2015-08-18 03:28 EDT, Manoj N Palat CLA
no flags Details | Diff
Proposed patch (8.49 KB, patch)
2015-08-26 08:41 EDT, Manoj N Palat CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Missing name CLA 2015-06-23 08:46:08 EDT
If a TypeBinding represents a parameterized type it can be checked with the isParameterizedType() method on ITypeBinding. For the intersection types in cast expressions there is no such check (like an: isIntersectionType() method). There also seems to be no way to access the different types in an intersection type binding.
Comment 1 Jay Arthanareeswaran CLA 2015-06-24 02:58:26 EDT
Manoj, please see what we can do if the concern is valid. Thanks!
Comment 2 Stephan Herrmann CLA 2015-06-24 04:54:48 EDT
See also bug 403917 (search for "intersection").
Comment 3 Manoj N Palat CLA 2015-08-18 03:28:30 EDT
Created attachment 255915 [details]
Proposed Patch wip

(In reply to Missing name from comment #0)
> If a TypeBinding represents a parameterized type it can be checked with the
> isParameterizedType() method on ITypeBinding. For the intersection types in
> cast expressions there is no such check (like an: isIntersectionType()
> method). There also seems to be no way to access the different types in an
> intersection type binding.

Jay/MissingName: To be precise, the request is for the dom equivalent for isIntersectionType18() binding check - the cast expression intersection types are represented in dom by dom.IntersectionType. The different types in the IntersectionType can be accessed via the method types(). The attached patch has isIntersectionTyp18() API added as the name for the api and I have put this as wip for now.

Stephan/Markus: Regarding different kinds, the concern is also found in bug 399792 comment 12. Bug 99931 becomes relevant in this case and we can probably think of another dom node for "normal" intersection types and leave the current IntersectionType to denote only casting expression intersectiontype (need to come up with a good name though). If that is agreed upon, I would commit this api and close this bug and take that up separately.
Comment 4 Markus Keller CLA 2015-08-18 11:50:21 EDT
ITypeBinding#isIntersectionType18() should not be released as API. If we add an API, then it must get a decent name and a Javadoc that tells how to fetch the intersected types. See e.g. ITypeBinding#isCapture() for how to do that.

Furthermore, as discussed in bug 99931, there's a lot more to do if you really want to add a new type bindings kind:
- document in ITypeBinding's Javadoc
- make sure the is*() getters for other kinds don't return true any more
- define/fix things like name, qualified name, binary name
- make a pass over all getters and document the new feature
- define a new format for the key
- make sure BindingKey, ASTParser etc. handle the new key

... and last but not least: consider introducing a new AST level, since some of these changes will inevitably break some clients.

There are very few use cases for intersection types in cast expressions. We shouldn't touch this without a good reason for why a new kind is essential.

Clients that really need to know more about such a binding can get the information already now. Check with the https://www.eclipse.org/jdt/ui/astview/
Comment 5 Stephan Herrmann CLA 2015-08-18 13:04:25 EDT
(In reply to Markus Keller from comment #4)
> ITypeBinding#isIntersectionType18() should not be released as API. If we add
> an API, then it must get a decent name

Let's try and see if we can get away with a single "isIntersectionType()", thus hiding all the differences made inside the compiler.

When true, the binding should answer to "ITypeBinding[] getIntersectingTypes()".

As for the name and similar we may have to live with the kludge of just taking the name of the first type.

In the compiler, intersection types occur as
- IntersectionTypeBinding18
  - from resolving an IntersectionCastTypeReference
  - as a result from type inference
  - as the erasure of a CaptureBinding18

Other types with possibly multiple bounds:
- WildcardBinding
- TypeVariableBinding
- CaptureBinding
- CaptureBinding18
For these the single "getBound()" is insufficient. In the compiler, WildcardBinding may also report its kind() as Binding.INTERSECTION_TYPE, which opens two options:
1. let the above types answer to getIntersectingTypes(), or,
2. add "TypeBinding[] getUpperBounds()"
(Saying "upper" because CaptureBinding18 may also have a lower bound, but I don't see any client for that piece of information - yet ...)
When opting for (1) it follows that isIntersectionType() isn't disjoint to neither isWildcardType() nor isCapture().

In the compiler some complication arises from the distinction of multiple super*interfaces* vs. multiple *class* bounds, but I don't think we'd want to make that difference at the level of DOM bindings.

Did I forget any?

Re binding keys: BindingKeyResolver already handles much of this, but we have bug 429264 for missing functionality regarding CaptureBinding18.


I don't see any need to change DOM AST, if anybody was thinking of that.
Comment 6 Manoj N Palat CLA 2015-08-26 08:41:08 EDT
Created attachment 256132 [details]
Proposed patch

(In reply to Stephan Herrmann from comment #5)
> (In reply to Markus Keller from comment #4)
> > ITypeBinding#isIntersectionType18() should not be released as API. If we add
> > an API, then it must get a decent name
> 
> Let's try and see if we can get away with a single "isIntersectionType()",
> thus hiding all the differences made inside the compiler.

Thanks Stephan & Markus for the comments. The attached patch implements the suggestion above.
> 
> WildcardBinding may also report its kind() as Binding.INTERSECTION_TYPE,
> which opens two options:
> 1. let the above types answer to getIntersectingTypes(), or,
Option 1 implemented.
Comment 7 Markus Keller CLA 2015-08-26 10:29:59 EDT
Instead of getIntersectingTypes(), we should just use getTypeBounds() for the upper bounds.
Comment 8 Manoj N Palat CLA 2015-08-26 11:06:25 EDT
(In reply to Markus Keller from comment #7)
> Instead of getIntersectingTypes(), we should just use getTypeBounds() for
> the upper bounds.

What's the rationale?
Comment 9 Markus Keller CLA 2015-08-26 11:20:37 EDT
> What's the rationale?

Simplicity and API consistency.
Comment 10 Eclipse Genie CLA 2015-08-27 01:03:45 EDT
New Gerrit change created: https://git.eclipse.org/r/54649
Comment 12 Manoj N Palat CLA 2015-08-27 05:52:28 EDT
(In reply to Eclipse Genie from comment #11)
> Gerrit change https://git.eclipse.org/r/54649 was merged to [master].
> Commit:
> http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/
> ?id=74bb5893666ff64fec3acee561b65f591ab9aed2

Markus: Have incorporated your comment in the commit. Thanks.
Comment 13 Markus Keller CLA 2015-08-27 14:35:42 EDT
What about all the other concerns in comment 4 and bug 99931?
Comment 14 Manoj N Palat CLA 2015-08-27 19:25:36 EDT
(In reply to Markus Keller from comment #13)
> What about all the other concerns in comment 4 and bug 99931?

Comment 4 talks about about getting a "decent" name and updating the javadoc in one part and other part talks about what to do when a new type binding is introduced.
This fix takes the solution suggested in comment 5 and addresses the issue of checking whether a binding represents intersection type or not and to get the involved types if it does without introducing a new type binding node. In this context, the only relevant parts of comment 4 were about getting a "decent" name and updating the javadoc. These two concerns are addressed already. Given this context what are the "other" concerns?
Comment 15 Stephan Herrmann CLA 2015-09-05 06:50:22 EDT
I can't comment regarding comment 13, but still a few remarks:

(1) We should coordinate how to handle CaptureBinding18:
Compiler perspective:
- may set multiple upper bounds that are classes
- order of class vs. interface bounds is not guaranteed
- some bounds may be missing when only looking at superclass and superInterfaces
DOM perspective:
- javadoc from getTypeBounds():
 "Note that per construction, it can only contain one class or array type, at most, and then it is located in first position."

If you want to relate CaptureBinding18 to the spec, please see the following sentence inside 18.4:
"let Y1, ..., Yn be fresh type variables whose bounds are as follows"
- during construction no restrictions on the kinds of bounds are made
- only via isConsistentIntersection() we are _checking_ if incompatible class bounds are present, but we do not currently remove redundant class bounds (compiler doesn't seem to need that).

I'm not sure whether elimination of redundant bounds should happen in the compiler or DOM. Probably best to try to do it right when initializing the binding, but need to check if that changes behaviour in the compiler. I've filed bug 476695 to investigate this road.


(2) I believe isIntersectionType() should answer true for more types, in particular captures with multiple bounds (see its javadoc).
BTW, the non-javadoc comment of TB.isIntersectionType() still refers to isIntersectionType18().
Comment 16 Markus Keller CLA 2015-09-14 20:29:36 EDT
(In reply to Manoj Palat from comment #14)
The ITypeBinding API says: "There are a number of different kinds of type bindings" and then lists the kinds.

Each kind has a corresponding is*() method, and these kinds are mutually exclusive. You can't just add a new is*() method and keep the result for another is*() query true. If you declare a new kind, then you have to go the full way. And that includes visible API changes for clients. And there, we have to see whether it's worth doing it now, or if we better wait for the next AST level, where it's OK to make breaking changes.
Comment 17 Timo Kinnunen CLA 2015-09-15 00:08:24 EDT
All type variable bounds create a corresponding intersection type. "This intersection type is often trivial, consisting of a single type" says the JLS, but technically that's still an intersection type.

isIntersectionType() && isTypeVariable() is a different type than isIntersectionType() && isCapture(), but it doesn't feel like these should be made separate APIs.

Maybe ITypeBinding should get a method asIntersectionType() to provide an intersection type specific view to the binding? The view could be extended separately if in the future Java ever gains the means to declare such types directly.
Comment 18 Manoj N Palat CLA 2015-09-15 06:31:50 EDT
Moving this to M3 so that a consensus can be arrived at.
Comment 19 Stephan Herrmann CLA 2015-09-15 07:32:38 EDT
(In reply to Markus Keller from comment #16)
> (In reply to Manoj Palat from comment #14)
> The ITypeBinding API says: "There are a number of different kinds of type
> bindings" and then lists the kinds.
> 
> Each kind has a corresponding is*() method, and these kinds are mutually
> exclusive. You can't just add a new is*() method and keep the result for
> another is*() query true. 

So we seem to be discussing whether isIntersectionType() defines (a) a kind of types or (b) a property of types (which can be true for different kinds of types). IMO it is wrong to imply that is*() methods can only be defined for (a). The implication can only be: if X is listed as a kind of types in ITypeBinding then there must be a corresponding isX(). I see no reason for the reverse implication.

This is not a difficulty which we created, but originates in JLS, which defines (a) intersection types as well as other kinds that (b) can be intersection types, too.

The other part of the equation is the exact contract of getTypeBounds():
It is an open question, whether we want/need API that exposes *all* constituents of an intersection type, possibly including more than one class, or whether redundant classes (superclasses of another class in the list) should be implicitly eliminated.

If the answer to that question is: "yes, we need to represent intersection types exactly as specified/computed, with support for multiple class constituents" then getTypeBounds() is not sufficient for intersection types, i.e., a new API getIntersectingTypes() is indeed necessary (contrary to comment 7).

If the answer is "no, filtered intersecting types with at most one class constituent are all we need", then the current implementation could be used as-is (with proper filtering to be added for CaptureBinding18), just more documentation is necessary:
- include Intersection Type in the list of kinds of types
- include a paragraph explaining that any t with t.getTypeBounds().length > 1 is an intersection type (even if t.isIntersectionType() answers false).
I.e. we need to thoroughly explain that there are two castes of intersection types, those with blessing from isIntersectionType() and those that shall not be named Intersection Type but are still intersection types.
In this universe we could say that isIntersectionType() simply fills the gap of those types that have getTypeBounds().length > 1 but are neither type variables, wildcards nor captures.
This all sounds very confusing to me and can only be simplified by admitting overlapping is*() methods.

At the end of the day we need to specifically decide, how to classify CaptureBinding18: those are (per JLS) "fresh type variables", in the same way as captures are "fresh type variables", and they can be "intersection types". We don't want to call them "typesIntroducedBy18_5_bullet6".
Comment 20 Manoj N Palat CLA 2015-10-26 00:55:04 EDT
Moving to M4 as M3 is round the corner - let us plan to discuss this in early M4 to reach a consensus and close
Comment 21 Manoj N Palat CLA 2015-12-04 23:00:17 EST
(In reply to Manoj Palat from comment #20)
> Moving to M4 as M3 is round the corner - let us plan to discuss this in
> early M4 to reach a consensus and close

Moving out of M4
Comment 22 Manoj N Palat CLA 2016-03-13 19:52:45 EDT
(In reply to Stephan Herrmann from comment #19)
> If the answer to that question is: "yes, we need to represent intersection
> types exactly as specified/computed, with support for multiple class
> constituents" then getTypeBounds() is not sufficient for intersection types,
> i.e., a new API getIntersectingTypes() is indeed necessary (contrary to
> comment 7).
> 

Markus: Do you have any specific comments here? We will have to push this into M6 if we decide to include the new API?
Comment 23 Stephan Herrmann CLA 2016-04-21 07:05:08 EDT
(In reply to Manoj Palat from comment #3)
> ... Bug 99931 becomes relevant in this case ...

What *is* the relationship between both bugs? Duplicates? Different solution strategies for the same requirement?
Comment 24 Manoj N Palat CLA 2018-04-16 09:30:31 EDT
bulk move out of 4.8