Initial proof of concept for implementation of -Xlint:hiddenclass

[Maurizio Cimadamore]

Excellent - I would add a test to check that the flag work under 
separate compilation environment.

Maurizio

On 20/09/12 13:23, Fredrik Öhrström wrote:
> 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
>>>>>>>