Community
Participate
Working Groups
We might want to evolve the build environment compiler from bug 417803 into a general purpose feature. Following bug 417803 comment 17 this could be done by introducing an annotation org.eclipse.jdt.annotation.Uninterned, technically this change might actually be pretty easy. Here are a few design options we need to decide: (a) positive or negative annotations: The http://types.cs.washington.edu/jsr308 site has a few mentionings of @Interned, quoting a study counting numbers of required annotations, among which: "1 annotation per 1736 lines for interning annotations (@Interned).". This boils down to the question, which default will require fewer annotations or produce fewer useless warnings. (b) type annotation or declaration annotation: Obviously the JSR308 site speaks of @Interned as a TYPE_USE annotation, but I'm not convinced that allowing different uses of the same type actually makes sense. Declaring a type as @Uninterned or @Interned is probably the normal use case. We only need to decide if we want to support exceptional usage requiring TYPE_USE refinement of how the type was actually declared. (c) inheritance (implicit vs. explicit): do we anticipate cases where types in the same inheritance hierarchy have different interning semantics? If so, we may want to require each subtype to explicitly state its (un)interning. Otherwise annotating a supertype can be used as an abbreviation for annotating an entire inheritance tree. Will overriding of an inherited annotation be legal? This should be seen in connection to (a). I think the literature will probably have the answers already. We just need to check if we believe these answers to be sufficiently pragmatic for large scale real world use.
a) and b) @Uninterned on a class that doesn't override equals() doesn't make much sense. Every instance of such class is equal to only itself and identity comparison between them is valid. This case should probably cause a warning about a missing equals-method. So @Uninterned on class declaration would be good documentation and the same reasoning works for @Interned on class declarations as well. For type use, an instance becoming @Uninterned would imply that it is now only to be compared using some Comparator procedure, for example. But the instances are still interned wrt. reference comparison, so this isn't very useful. An uninterned object gaining @Interned by going thru a custom interning method is a much more familiar use-case. In summary then, @Uninterned on declaration only, @Interned on both declaration and use, IMO. c) One such hierarchy is Number. Integer.valueOf() for compile-time constant arguments from -128 to 127 always returns interned objects, but BigInteger.valueOf() doesn't guarantee the same. And lest we be tempted to dismiss this case outright, being able to get a warning from code like this would be cool: Integer a1 = ...; Integer a2 = 127; Integer a3 = 255; if(a1 == a2 || a1 == a3) { // a1==a3 is by default always false System.out.println("yes"); }
See also https://bugs.eclipse.org/bugs/show_bug.cgi?id=420440
With the plan to move JDT/Core builds to Java 6, we may able to able to work on a minimal cut of this sooner than originally thought. I would still want to wait for the full blown work to be taken up only after Java 8 ships, but if we carefully design the annotation(s) involved, we can convert the existing implementation to use annotations rather than hard coding type names - taking it very close to the final implementation
We should also keep resolutions for such warnings in mind. The patch in bug 419444 hardcodes the fully qualified names of the methods to use instead of == and !=. Ideally, the method(s) could be specified on the annotation, so that a quick fix can automatically replace existing comparisons.
(In reply to Markus Keller from comment #4) > We should also keep resolutions for such warnings in mind. The patch in bug > 419444 hardcodes the fully qualified names of the methods to use instead of > == and !=. Ideally, the method(s) could be specified on the annotation, so > that a quick fix can automatically replace existing comparisons. Good point. In the interim, I suggested to Noopur that the hardcoded names be toggled based on whether it is ITypeBinding's or (compiler) TypeBinding's that are being compared. We found all of 3 places in JDT/Core where ITypeBindings were being compared (and those were also correct comparisons). So this suggestion is really for the UI work.
(In reply to Markus Keller from comment #4) > We should also keep resolutions for such warnings in mind. The patch in bug > 419444 hardcodes the fully qualified names of the methods to use instead of > == and !=. Ideally, the method(s) could be specified on the annotation, so > that a quick fix can automatically replace existing comparisons. Wouldn't the "normal" application be to convert o1 == o2 into o1.equals(o2) ? Hm, works if 'o1' is @NonNull, but ... But are static equals methods really so common?
(In reply to comment #6) > Wouldn't the "normal" application be to convert > o1 == o2 > into > o1.equals(o2) > ? Objects.equals(o1, o2) would be better.
(In reply to Timo Kinnunen from comment #7) > Objects.equals(o1, o2) would be better. oops, sure. Thanks for the hint. Only downside: @since 1.7
(In reply to Srikanth Sankaran from comment #5) > So this suggestion is really for the UI work. This bug is about turning the internal compiler option into a more generally useful story. I mentioned this sub-problem here, since the chosen solution will decide whether we can implement a generally useful quick fix or not.
(In reply to Markus Keller from comment #9) > (In reply to Srikanth Sankaran from comment #5) > > So this suggestion is really for the UI work. > > This bug is about turning the internal compiler option into a more generally > useful story. I mentioned this sub-problem here, since the chosen solution > will decide whether we can implement a generally useful quick fix or not. The "suggestion" I was referring is *mine* to Noopur mentioned in comment#5 and not yours from comment#4.
*** Bug 39095 has been marked as a duplicate of this bug. ***
(In reply to Palmer Eldritch from bug 39095 comment #12) > Please consider this (and consider upping the vote count :). It does happen > from time to time, Intellij and netbeans have it and it is as much a > warning as anything - at least for strings. *If* we opt for @Uninterned, then String should definitely be hard-coded as @Uninterned even without the annotation.
