Initial proof of concept for implementation of -Xlint:hiddenclass

[Fredrik Öhrström]

Third webrev: http://cr.openjdk.java.net/~ohrstrom/webrev-7153951-v3

2012/9/13 Jonathan Gibbons <jonathan.gibbons at oracle.com>:
> The arity of the message is wrong. I Check 3111, you provide 3 args.
> In the resource file you only expect 2.
>
> -- Jon
>
>
> On 09/12/2012 07:07 PM, Jonathan Gibbons wrote:
>>
>> In the message file, the comment on 1721 is redundant since the same info
>> is on
>> line 1722.   These comments are used by the localization team. They are in
>> a stylized form
>> so that they can be mechanically checked by utilities in the
>> test/tools/diags/examples world.
>>
>> Since there is a new message, you will need a new example, in order to
>> pass the tests.
>>
>> Check:3107
>> The convention is to use the kind, not use instanceof
>>
>> -- Jon
>>
>> On 09/13/2012 08:06 AM, Maurizio Cimadamore wrote:
>>>
>>> On 12/09/12 16:00, Fredrik Öhrström wrote:
>>>>
>>>> Second round of implementation:
>>>> http://cr.openjdk.java.net/~ohrstrom/webrev-7153951-v2/
>>>> <http://cr.openjdk.java.net/%7Eohrstrom/webrev-7153951-v2/>
>>>>
>>>> When building the openjdk, it outputs 316 warnings like these:
>>>> .../ClientHandshaker.java:723: warning: secondary class
>>>> SupportedEllipticCurvesExtension in …./HelloExtensions.java should not be
>>>> accessed from outside its own source file
>>>
>>> Great work - couple of comments:
>>>
>>> *) it seems like the warning should not be generated when isAccessible
>>> fails, so, why not using that method as a condition to filter out the
>>> warning (instead of inlining that logic in the method guard) ?
>>>
>>> *) is there a better name than 'secondary class'  (maybe a question for
>>> our spec gurus?)
>>>
>>> Maurizio
>>>
>>>>
>>>>
>>>> //Fredrik
>>>>
>>>> 16 mar 2012 kl. 16:03 skrev Jonathan Gibbons:
>>>>
>>>>> Fredrik,
>>>>>
>>>>> Good start.
>>>>>
>>>>> 1. It should be sufficient to remove references to the file manager and
>>>>> use something like
>>>>>
>>>>>    c.sourcefile == env.toplevel.sourcefile
>>>>>
>>>>> to check if the class is in the "right" file.
>>>>>
>>>>> 2. I think line numbers are desirable, and would be easy if you move
>>>>> the checkForHiddenAccess from Resolve into Check, and call it from Attr.
>>>>>
>>>>> 3. I think a warning per reference is acceptable: it is not necessary
>>>>> to optimize it to a warning per hidden class.
>>>>>
>>>>> 4. We'll need a CCC approval for the new Xlint suboption.
>>>>>
>>>>> 5. We should check to see if there is a preferred term for what you
>>>>> call "hidden class".
>>>>>
>>>>> -- Jon
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 03/16/2012 05:12 AM, Fredrik Öhrström wrote:
>>>>>>
>>>>>> > From the bug:
>>>>>>
>>>>>> Although legal, the use of multiple top level classes in the same file
>>>>>> is somewhat questionable to begin with, but it is particularly bad
>>>>>> when
>>>>>> in some package class A in A.java refers to class B defined in C.java.
>>>>>> This requires that at times the files must be compiled together, and
>>>>>> prevents implicit compilation from locating such "hidden classes".
>>>>>>
>>>>>> Proof of concept impl:
>>>>>> http://cr.openjdk.java.net/~ohrstrom/webrev-7153951-v1/
>>>>>> <http://cr.openjdk.java.net/%7Eohrstrom/webrev-7153951-v1/>
>>>>>>
>>>>>> It seems to detect the problem correctly. And there are a few of these
>>>>>> in the jdk, it seems, ~100.
>>>>>>
>>>>>> How to do isSameFile test properly?
>>>>>> How to report each hidden class reference only once?
>>>>>> Are line numbers neccesary?
>>>>>> Where to put checkForHiddenAccess?
>>>>>> Does it cover all possible use cases?
>>>>>> What else did I miss?
>>>>>>
>>>>>> //Fredrik
>>>>>>
>>>>>
>>>>
>>>
>>
>