Review request for JDK-8013357: Javac accepts erroneous binary comparison operations

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Thu Jun 27 01:28:32 PDT 2013


On 13/06/13 21:02, Eric McCorkle wrote:
> Thanks Alex.  Your suggested comment changes will be applied.
>
> Given that this patch changes the behavior of the type checker, though,
> I would like to get approval from Jon or Maurizio (if not both) before
> it goes in.
I'm good with the patch - I notice the test is still a standalone test 
that doesn't make use of the types harness - but I think that's ok, as 
the kind of checks you do is broader than just testing the new routine 
you introduced. I wonder if we should also add a unit test (as a types 
harness subtest) for the new routine, just to make sure it behaves as 
expected.

Maurizio
>
> On 06/13/13 15:41, Alex Buckley wrote:
>> With regard to TestComparisons.java in
>> http://cr.openjdk.java.net/~emc/8013357/webrev.06/ :
>>
>> - In the second set of truth tables (for boxed primitive types), I like
>> how the 3 moves across the second row :-)
>>
>> - The comment "// Reference types" should be something like "//
>> Important reference types", because the "// Boxed primitives" comment
>> also deals with reference types.
>>
>> - Line 167 says Int but means Integer.
>>
>> - Line 271/272 should be "String, comparable to any reference type,
>> excluding those convertible to a numeric types, but including any
>> captures."
>>
>> Other than that, good to go.
>>
>> Alex
>>
>> On 6/13/2013 12:28 PM, Eric McCorkle wrote:
>>> I've moved the new test into types as Jon suggested, and reordered the
>>> lists/truth tables as Alex suggested.  A new webrev is up here:
>>> http://cr.openjdk.java.net/~emc/8013357/
>>>
>>> On 06/11/13 12:31, Jonathan Gibbons wrote:
>>>> On 06/11/2013 06:10 AM, Eric McCorkle wrote:
>>>>> Good suggestion.
>>>>>
>>>>> Something else occurred to me: the scope of this test has grown beyond
>>>>> that of a simple regression test.  It should probably be moved to a
>>>>> subdirectory of test/tools/javac purposed for more exhaustive tests.
>>>>> Jon or Maurizio, do you have an suggestions?
>>>> Yes, test/tools/javac/types seems a better home.
>>>>
>>>> We are trying to deprecate/minimize the use of bug numbers within
>>>> the test/tools/javac directory itself.
>>>>
>>>> -- Jon



More information about the compiler-dev mailing list