attributing files with syntax errors

[Liam Miller-Cushon]

Thanks for taking a look.

On Thu, Aug 20, 2015 at 6:04 PM, Jonathan Gibbons <
jonathan.gibbons at oracle.com> wrote:

> Twiddling the CompilePolicy is an "internal" knob, controlled by a
> "hidden" (-XD) option. That being said, my primary concern about changing
> the bytodo policy is that it is the default policy, so changing it will
> potentially affect everyone, even those who don't know about the
> CompilePolicy.
>

Understood. It's possible that showing errors from attr is actually better
for most people, since the extra information could be helpful when fixing
compilation errors. One way to minimize the impact would be to change the
default shouldStopPolicyIfError mode to attr at the same time the bug was
fixed, so the default behaviour was unchanged.


> And _that_ being said, the original intent of providing the CompilePolicy
> was to address bugs in which a file had multiple top level classes, and
> some could have classes generated before errors were detected in other
> classes in the same file.  Hence the byfile policy. But for various
> reasons, byfile never became the default. I seem to recall it was at least
> in part due to memory usage, which is less of a concern today than it was
> back then.


> The other area where this might be relevant is annotation processing,
> where the original code was not so good back then at differentiating
> different types of errors, and so we wanted compilation to continue more
> often than we do now. Nowadays, javac has a better concept of "recoverable
> errors", and so javac is better at not proceeding unnecessarily.
>

Interesting. The reason we came across this and were experimenting with
byfile is for error prone. It's occasionally useful to inspect an entire
compilation unit instead of individual top-level declarations, and with the
bytodo policy top-level declarations are lowered one at time, so by the
time everything in a compilation unit has gone through flow some things
have already been lowered. (I think the plugin api makes it harder to
observe that by associating events with top-level declarations, and
discouraging inspecting compilation units.)


> So I guess I'd be interested to see the patch, although to prevent
> unnecessary work up front, it may be worth posting the src/ patch first,
> with some info on just how many and which tests need fixing.   The more
> tests that need fixup, the more carefully we should review the change in
> behavior.
>

Here's the trivial fix I was testing, which just replicates the logic in
byfile for todo. I realized after doing this that the same issue affects
all of the other policies, so the check should probably be moved higher up.

diff -r eaab8a16dcfb
src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java
---
a/src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java
Thu Aug 20 11:38:24 2015 -0700
+++
b/src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java
Thu Aug 20 18:09:00 2015 -0700
@@ -867,7 +867,7 @@
                 break;

             case BY_TODO:
-                while (!todo.isEmpty())
+                while (!todo.isEmpty() && !shouldStop(CompileState.ATTR))
                     generate(desugar(flow(attribute(todo.remove()))));
                 break;

I counted 37 test failures, excluding two that test compilation policies.
The list is attached.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20150820/f49c538d/attachment-0001.html>
-------------- next part --------------
Ambig3.java
FlatnameClash2.java
NestedInnerClassNames.java
OverridePosition.java
T6326754.java
4980495/std/Test.java
6304921/T6304921.java
7086595/T7086595.java
8052070/DuplicateTypeParameter.java
ExtendsAccess/ExtendsAccess.java
QualifiedAccess/QualifiedAccess_1.java
QualifiedAccess/QualifiedAccess_2.java
annotations/neg/NoDefault.java
annotations/neg/NoStatic.java
annotations/repeatingAnnotations/MissingDefaultCase1.java
annotations/repeatingAnnotations/MissingDefaultCase2.java
annotations/repeatingAnnotations/MissingValueMethod.java
annotations/repeatingAnnotations/UseWrongRepeatable.java
annotations/repeatingAnnotations/WrongReturnTypeForValue.java
annotations/repeatingAnnotations/8029017/TypeUseTargetNeg.java
annotations/typeAnnotations/DeclVsUseErrorMessage.java
defaultMethods/Neg13.java
defaultMethods/private/Private02.java
defaultMethods/private/Private06.java
defaultMethods/private/Private08.java
enum/Enum2.java
enum/T5081785.java
generics/InheritanceConflict3.java
generics/inference/4972073/T4972073.java
generics/rawOverride/7157798/Test3.java
generics/rawOverride/7157798/Test4.java
lambda/FunctionalInterfaceAnno.java
lambda/LambdaConv18.java
lambda/funcInterfaces/NonSAM2.java
protectedAccess/ProtectedMemberAccess2.java
protectedAccess/ProtectedMemberAccess3.java
staticImport/ImportPrivate.java