Error count propagation between rounds

Jonathan Gibbons jonathan.gibbons at oracle.com
Mon Nov 26 11:35:24 PST 2012 

Superficially, the changes look good -- so good that I'm trying them out 
as a candidate for a separate fix in TL/langtools.

However, as good as they are, I think they can be made better. By fixing 
the bugs you have found, I think you have eliminated the last remaining 
reasons why we use a separate instance of a Log for each round of 
processing.   Therefore, I think it may be possible to eliminate 
Log.initRound, and simply propogate the single Log object from round to 
round, as exemplified in JavacProcessingEnvironment.Round.nextContext(), 
such as this:

             Tokens tokens = Tokens.instance(context);
             Assert.checkNonNull(tokens);
             next.put(Tokens.tokensKey, tokens);

I'm trying that out now.

-- Jon

On 11/19/2012 02:14 AM, Werner Dietl wrote:
> Jon, all,
>
> please have a look at:
>
> http://hg.openjdk.java.net/type-annotations/type-annotations/langtools/rev/8466f6bcab55
>
> I made some changes to JavacProcessingEnvironment and needed to
> adapted only a few test cases.
> To me the changes in the test cases make sense.
>
> Our build currently has 4 errors, see:
>
> http://buffalo.cs.washington.edu:8080/job/type-annotations-langtools/lastBuild/console
>
> T8000484.java and CheckExamples.java seem to be related to Lambda.
> Only TestSmoke.java is related to JSR 308. It will be resolved after
> the bytecode changes.
> That is, all existing tests for annotation processing and error
> propagation pass with this change.
>
> Please do let me know what you think of the error propagation changes.
>
> cu, WMD.
>
>
>
> On Wed, Nov 14, 2012 at 9:32 PM, Jonathan Gibbons
> <jonathan.gibbons at oracle.com> wrote:
>> OK, let me know.
>>
>> -- Jon
>>
>>
>>
>> On 11/14/2012 09:30 PM, Werner Dietl wrote:
>>> Jon,
>>>
>>> I'm not quite sure simply propagating nerrors is working correctly
>>> yet. Sometimes this causes errors to be counted multiple times, it
>>> seems.
>>>
>>> How about the following: I go through all failures and for each see
>>> whether I think the expected output should be fixed or whether
>>> something else needs work.
>>> If I think the output needs adapting, I'll push such changes.
>>> I'll send a message with the test cases where I'm not happy with the
>>> results and we can discuss how to proceed.
>>>
>>> Thanks,
>>> cu, WMD.
>>>
>>> On Wed, Nov 14, 2012 at 9:13 PM, Jonathan Gibbons
>>> <jonathan.gibbons at oracle.com> wrote:
>>>> Werner,
>>>>
>>>> Yes, at the time that we did a lot of cleanup in
>>>> JavacProcessingEnvironment
>>>> in JDK 7, we were not significantly using the technique of propogating so
>>>> much information between rounds, and it led to the curiousity you've
>>>> observed where the identical error was reported twice and counted once,
>>>> with
>>>> no apparent solution at that time.
>>>>
>>>> Now that we are propogating more info between rounds, your solution is
>>>> elegant and the one I would have suggested myself.
>>>>
>>>> I would suggest that this is applied as a standalone fix to TL and pulled
>>>> down into the type-annotations forest. Do you want to create that patch,
>>>> or
>>>> should I?
>>>>
>>>> -- Jon
>>>>
>>>>
>>>>
>>>> On 11/14/2012 08:54 PM, Werner Dietl wrote:
>>>>> Jon,
>>>>>
>>>>> thanks for that update.
>>>>> This sounds like you agree that the behavior I described with Java 7
>>>>> is incorrect and should be fixed.
>>>>> Will somebody from the javac group suggest a better fix than what I
>>>>> did in type-annotations?
>>>>>
>>>>> Thanks,
>>>>> cu, WMD.
>>>>>
>>>>> On Mon, Nov 12, 2012 at 12:05 PM, Jonathan Gibbons
>>>>> <jonathan.gibbons at oracle.com> wrote:
>>>>>> Currently, the policy is that unrecoverable errors should be reported,
>>>>>> and
>>>>>> go to the error count.  They should cause round processing to stop.  It
>>>>>> may
>>>>>> be the case that some errors are reported twice, which would be a bug,
>>>>>> separate from the total number of bugs reported.
>>>>>>
>>>>>> Recoverable errors should be suppressed, and do not terminate round
>>>>>> processing.
>>>>>>
>>>>>> Generally speaking, only "name not found" errors are classified as
>>>>>> recoverable errors, since they may be generated by additional rounds of
>>>>>> processing.
>>>>>>
>>>>>> -- Jon
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 11/12/2012 11:11 AM, Werner Dietl wrote:
>>>>>>> Dear all,
>>>>>>>
>>>>>>> in one of my recent pushes, in particular, in:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> http://hg.openjdk.java.net/type-annotations/type-annotations/langtools/rev/b7f384a554b3
>>>>>>>
>>>>>>> I introduced that the nerrors information is propagated between
>>>>>>> rounds.
>>>>>>>
>>>>>>> This fixed three test case errors I've been having, but also
>>>>>>> introduced 19 new ones.
>>>>>>> I would like to discuss what the best solution to this issue is.
>>>>>>>
>>>>>>> To illustrate my point, let us take stock Java 7 (javac 1.7.0_09) and
>>>>>>> JDK 8 Build b63 and the two attached files, which are based on
>>>>>>> langtools/test/tools/javac/api/6406133.
>>>>>>>
>>>>>>> (I ignore the single warning that is caused by the processor version
>>>>>>> mismatch. The result is the same if I adapt the version number for
>>>>>>> each compiler.)
>>>>>>>
>>>>>>> Let's first compile the processor, without output:
>>>>>>> javac SimpleProc.java
>>>>>>>
>>>>>>> Then, let's normally compile file Erroneous.java:
>>>>>>> $ javac Erroneous.java
>>>>>>> Erroneous.java:2: error: cannot find symbol
>>>>>>> class A extends Missing {
>>>>>>>                     ^
>>>>>>>       symbol: class Missing
>>>>>>> 1 error
>>>>>>>
>>>>>>> I would say that's the expected output.
>>>>>>>
>>>>>>> Now, let's add the annotation processor:
>>>>>>>
>>>>>>> $ javac -processor SimpleProc Erroneous.java
>>>>>>> Processing: [java.lang.Deprecated]
>>>>>>> Processing: []
>>>>>>> Erroneous.java:2: error: cannot find symbol
>>>>>>> class A extends Missing {
>>>>>>>                     ^
>>>>>>>       symbol: class Missing
>>>>>>> Erroneous.java:2: error: cannot find symbol
>>>>>>> class A extends Missing {
>>>>>>>                     ^
>>>>>>>       symbol: class Missing
>>>>>>> 1 error
>>>>>>>
>>>>>>> Isn't it strange that we get two error outputs and just one error
>>>>>>> count?
>>>>>>> The return value $? is 1, which is expected for an error.
>>>>>>>
>>>>>>> Finally, if we compile with "-proc:only" we get:
>>>>>>>
>>>>>>> $ javac -processor SimpleProc -proc:only Erroneous.java
>>>>>>> Processing: [java.lang.Deprecated]
>>>>>>> Processing: []
>>>>>>> Erroneous.java:2: error: cannot find symbol
>>>>>>> class A extends Missing {
>>>>>>>                     ^
>>>>>>>       symbol: class Missing
>>>>>>>
>>>>>>> We get only one error output, but suddenly no longer the "1 error"
>>>>>>> output. Also, $? is 0, indicating that there was no error.
>>>>>>> With -proc:only maybe semantic errors shouldn't be reported, because
>>>>>>> that phase is after processing. However, then I would not expect to
>>>>>>> see an error message.
>>>>>>>
>>>>>>> The order of the output is different between a JDK 6 and JDK 7, but
>>>>>>> the basic behavior is the same.
>>>>>>>
>>>>>>> With JDK 8 Build b63 from Nov 1 I get the same behavior as with Java
>>>>>>> 7, that is, 2 error messages but a failed compile with a processor, a
>>>>>>> single error message and a successful compile with proc:only.
>>>>>>>
>>>>>>> With my pushed changes, we get the following behavior:
>>>>>>>
>>>>>>> javac -cp . -processor SimpleProc Erroneous.java
>>>>>>> Processing: [java.lang.Deprecated]
>>>>>>> Processing: []
>>>>>>> Erroneous.java:25: error: cannot find symbol
>>>>>>> class A extends Missing {
>>>>>>>                     ^
>>>>>>>       symbol: class Missing
>>>>>>> 1 error
>>>>>>> $? is 1
>>>>>>>
>>>>>>> javac -cp . -processor SimpleProc -proc:only Erroneous.java
>>>>>>> Processing: [java.lang.Deprecated]
>>>>>>> Processing: []
>>>>>>> Erroneous.java:25: error: cannot find symbol
>>>>>>> class A extends Missing {
>>>>>>>                     ^
>>>>>>>       symbol: class Missing
>>>>>>> 1 error
>>>>>>> $? is 1
>>>>>>>
>>>>>>> The difference is made by the line:
>>>>>>>                 this.nerrors = other.nerrors;
>>>>>>> in
>>>>>>> com.sun.tools.javac.util.Log.initRound(Log)
>>>>>>>
>>>>>>> With setting nerrors we get:
>>>>>>>
>>>>>>>         [jtreg] Test results: passed: 2,409; failed: 19
>>>>>>>
>>>>>>> Without setting nerros we get:
>>>>>>>
>>>>>>>         [jtreg] Test results: passed: 2,425; failed: 3
>>>>>>>
>>>>>>> I don't know how to better fix the 3 remaining errors without setting
>>>>>>> nerrors. They are caused by executing Flow processing with an
>>>>>>> inconsistent AST, resulting in NPEs.
>>>>>>>
>>>>>>> I think some of the 19 errors can be fixed by admitting the "x errors"
>>>>>>> output in the expected file.
>>>>>>> Simply setting nerrors however seems to be incorrect in general, as it
>>>>>>> causes errors to be counted multiple times.
>>>>>>> Is there a way to decide whether to propagate the error count or not?
>>>>>>> Should -proc:only make a difference in this decision?
>>>>>>>
>>>>>>> All comments for how to better solve this problem would be very
>>>>>>> welcome.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> cu, WMD.
>>>>>>>
>>>
>
>

More information about the type-annotations-dev mailing list