Error count propagation between rounds

[Jonathan Gibbons]

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