Review request for JDK-8030642: Add golden files to javac/limits

Jonathan Gibbons jonathan.gibbons at oracle.com
Tue Dec 17 13:31:07 PST 2013


As another general comment; I encourage anyone working on javac 
diagnostics to run the "diags-examples" target in 
langtools/make/build.xml to see what non-raw diagnostics look like in 
full context.   Maybe it doesn't apply here so much, since you are not 
creating new diagnostics, just testing them better, but the general 
comment still stands.

-- Jon

On 12/17/2013 01:27 PM, Jonathan Gibbons wrote:
> Looks mostly OK.
>
> One important thing to consider in tests like this is whether the 
> messages are as good as they can/should be.   In this case, I am 
> reminded that there is another bug in the system to make all limit 
> messages more friendly, by including the actual value of the limit.  
> Right now the messages are all of the simple form "you have exceeded 
> the limit".  What you have done is OK, but you might want to look at 
> updating javac, the messages, and the tests, to dynamically include 
> the value of the limit in the message. (Meaning, the value should come 
> from the same place/constant as used in javac, and should not be 
> hard-wired into the message itself.).
>
> I had to check the full non-raw form of the diag for string.overflow, 
> but that one looks OK, along with the rest.
>
> Approved.
>
> -- Jon (Reviewer)
>
> On 12/17/2013 12:32 PM, Eric McCorkle wrote:
>> Approved, though I am not a capital-R reviewer.
>>
>> On 12/17/13 15:23, Paul Govereau wrote:
>>> Hello,
>>>
>>> Please review this patch adding expected output files for the negative
>>> tests in langtools/test/tools/javac/limits.
>>>
>>> The bug report for this patch is here:
>>> https://bugs.openjdk.java.net/browse/JDK-8030642
>>>
>>> The webrev is here:
>>> http://cr.openjdk.java.net/~emc/8030642/
>>>
>>> Thank you,
>>> Paul
>



More information about the compiler-dev mailing list