From 0a29628b6087360e13e38c2eea3c28b385352b69 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Tue, 24 Oct 2023 10:41:46 -0700 Subject: [PATCH] Fix a long-standing bug with type annotation paths on raw inner class types As described in [JVMS 4.7.20.2](https://docs.oracle.com/javase/specs/jvms/se21/html/jvms-4.html#jvms-4.7.20.2), type annotations on nested types are located with `type_path_kind=1` type path entries, which describe steps starting with the 'outermost part of the type for which a type annotation is admissible'. Turbine represents class types for inner classes as a flat list of 'simple class types' corresponding to outer and inner classes. The 'canonicalization' step normalizes type arguments, and also ensures there's exactly one entry for each nested class and non-static containing class. The former doesn't apply to raw types (we erase the entire type if any part of it is raw), but the latter is important to ensure we compute the right number of type path steps for inner classes. PiperOrigin-RevId: 576191070 --- .../google/turbine/types/Canonicalize.java | 6 ++-- .../turbine/lower/LowerIntegrationTest.java | 7 ++++ .../lower/testdata/type_anno_nested.test | 23 +++++++++++++ .../testdata/type_anno_nested_generic.test | 21 ++++++++++++ .../lower/testdata/type_anno_nested_raw.test | 32 +++++++++++++++++++ 5 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 javatests/com/google/turbine/lower/testdata/type_anno_nested.test create mode 100644 javatests/com/google/turbine/lower/testdata/type_anno_nested_generic.test create mode 100644 javatests/com/google/turbine/lower/testdata/type_anno_nested_raw.test diff --git a/java/com/google/turbine/types/Canonicalize.java b/java/com/google/turbine/types/Canonicalize.java index f944bb51..4739ecd5 100644 --- a/java/com/google/turbine/types/Canonicalize.java +++ b/java/com/google/turbine/types/Canonicalize.java @@ -124,9 +124,6 @@ private ClassTy canon(ClassSymbol base, ClassTy ty) { if (ty.sym().equals(ClassSymbol.ERROR)) { return ty; } - if (isRaw(ty)) { - return Erasure.eraseClassTy(ty); - } // if the first name is a simple name resolved inside a nested class, add explicit qualifiers // for the enclosing declarations Iterator it = ty.classes().iterator(); @@ -140,6 +137,9 @@ private ClassTy canon(ClassSymbol base, ClassTy ty) { while (it.hasNext()) { canon = canonOne(canon, it.next()); } + if (isRaw(canon)) { + canon = Erasure.eraseClassTy(canon); + } return canon; } diff --git a/javatests/com/google/turbine/lower/LowerIntegrationTest.java b/javatests/com/google/turbine/lower/LowerIntegrationTest.java index 7a6f0ffb..f77f2577 100644 --- a/javatests/com/google/turbine/lower/LowerIntegrationTest.java +++ b/javatests/com/google/turbine/lower/LowerIntegrationTest.java @@ -41,6 +41,10 @@ import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; +/** + * A test that compiles inputs with both javac and turbine, and asserts that the output is + * equivalent. + */ @RunWith(Parameterized.class) public class LowerIntegrationTest { @@ -314,6 +318,9 @@ public static Iterable parameters() { "type_anno_c_array.test", "type_anno_cstyle_array_dims.test", "type_anno_hello.test", + "type_anno_nested.test", + "type_anno_nested_generic.test", + "type_anno_nested_raw.test", "type_anno_order.test", "type_anno_parameter_index.test", "type_anno_qual.test", diff --git a/javatests/com/google/turbine/lower/testdata/type_anno_nested.test b/javatests/com/google/turbine/lower/testdata/type_anno_nested.test new file mode 100644 index 00000000..939329a1 --- /dev/null +++ b/javatests/com/google/turbine/lower/testdata/type_anno_nested.test @@ -0,0 +1,23 @@ +=== Annotations.java === +import static java.lang.annotation.ElementType.TYPE_USE; +import java.lang.annotation.Target; + +@Target(TYPE_USE) @interface A {} +@Target(TYPE_USE) @interface B {} +@Target(TYPE_USE) @interface C {} + +=== Outer.java === +class Outer { + + @A Outer . @B Middle . @C Inner f; + Outer . @A MiddleStatic . @B Inner g; + Outer . MiddleStatic . @A InnerStatic h; + + class Middle { + class Inner {} + } + static class MiddleStatic { + class Inner {} + static class InnerStatic {} + } +} \ No newline at end of file diff --git a/javatests/com/google/turbine/lower/testdata/type_anno_nested_generic.test b/javatests/com/google/turbine/lower/testdata/type_anno_nested_generic.test new file mode 100644 index 00000000..fcfe040c --- /dev/null +++ b/javatests/com/google/turbine/lower/testdata/type_anno_nested_generic.test @@ -0,0 +1,21 @@ +=== Annotations.java === +import static java.lang.annotation.ElementType.TYPE_USE; +import java.lang.annotation.Target; + +@Target(TYPE_USE) @interface A {} +@Target(TYPE_USE) @interface B {} +@Target(TYPE_USE) @interface C {} +@Target(TYPE_USE) @interface D {} + +=== Outer.java === +class Outer { + Outer . Middle<@A Foo . @B Bar> . Inner<@D String @C []> f; + + class Middle { + class Inner {} + } + + class Foo { + class Bar {} + } +} \ No newline at end of file diff --git a/javatests/com/google/turbine/lower/testdata/type_anno_nested_raw.test b/javatests/com/google/turbine/lower/testdata/type_anno_nested_raw.test new file mode 100644 index 00000000..e3fd0820 --- /dev/null +++ b/javatests/com/google/turbine/lower/testdata/type_anno_nested_raw.test @@ -0,0 +1,32 @@ +=== Annotations.java === +import static java.lang.annotation.ElementType.TYPE_USE; +import java.lang.annotation.Target; + +@Target(TYPE_USE) @interface A {} +@Target(TYPE_USE) @interface B {} +@Target(TYPE_USE) @interface C {} + +=== Outer.java === +import java.util.List; + +class Outer { + static class StaticMiddle { + class Inner {} + static class StaticInner {} + + // raw types with parameterized enclosing types + @A Inner a; + @A StaticInner b; + } + + Outer . StaticMiddle . @A Inner e; + Outer . StaticMiddle . @A StaticInner f; + Outer . StaticMiddle<@A String> . @B Inner<@C String> g; + + Outer . StaticMiddle<@A List> . @B Inner<@C List> h; + List i; + + // javac rejects these partially raw types + // Outer . StaticMiddle<@A String> . @B Inner j; + // Outer . StaticMiddle . @B Inner<@C String> k; +}