Initial proof of concept for implementation of -Xlint:hiddenclass

Jonathan Gibbons jonathan.gibbons at oracle.com
Wed Sep 12 19:09:27 PDT 2012


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
>>>>>
>>>>
>>>
>>
>




More information about the compiler-dev mailing list