changeset 59609:bb04a191551d

8236682: Javac generates a redundant FieldRef constant for record fields Reviewed-by: mcimadamore
author vromero
date Tue, 14 Jan 2020 21:49:43 -0500
parents d8a27d799478
children 14c78683c9f0
files src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java test/langtools/lib/combo/tools/javac/combo/CompilationTestCase.java test/langtools/tools/javac/records/RecordCompilationTests.java
diffstat 3 files changed, 59 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java	Tue Jan 14 23:40:42 2020 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java	Tue Jan 14 21:49:43 2020 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -2259,12 +2259,15 @@
     }
 
     List<JCTree> generateMandatedAccessors(JCClassDecl tree) {
+        List<JCVariableDecl> fields = TreeInfo.recordFields(tree);
         return tree.sym.getRecordComponents().stream()
                 .filter(rc -> (rc.accessor.flags() & Flags.GENERATED_MEMBER) != 0)
                 .map(rc -> {
+                    // we need to return the field not the record component
+                    JCVariableDecl field = fields.stream().filter(f -> f.name == rc.name).findAny().get();
                     make_at(tree.pos());
                     return make.MethodDef(rc.accessor, make.Block(0,
-                            List.of(make.Return(make.Ident(rc)))));
+                            List.of(make.Return(make.Ident(field)))));
                 }).collect(List.collector());
     }
 
--- a/test/langtools/lib/combo/tools/javac/combo/CompilationTestCase.java	Tue Jan 14 23:40:42 2020 +0100
+++ b/test/langtools/lib/combo/tools/javac/combo/CompilationTestCase.java	Tue Jan 14 21:49:43 2020 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,7 @@
  */
 package tools.javac.combo;
 
+import java.io.File;
 import java.io.IOException;
 
 import org.testng.ITestResult;
@@ -69,28 +70,34 @@
         return s;
     }
 
-    private void assertCompile(String program, Runnable postTest) {
+    private File assertCompile(String program, Runnable postTest, boolean generate) {
         reset();
         addCompileOptions(compileOptions);
         addSourceFile(defaultFileName, program);
+        File dir = null;
         try {
-            compile();
+            dir = compile(generate);
         }
         catch (IOException e) {
             throw new RuntimeException(e);
         }
         postTest.run();
+        return dir;
     }
 
     protected void assertOK(String... constructs) {
-        assertCompile(expandMarkers(constructs), this::assertCompileSucceeded);
+        assertCompile(expandMarkers(constructs), this::assertCompileSucceeded, false);
+    }
+
+    protected File assertOK(boolean generate, String... constructs) {
+        return assertCompile(expandMarkers(constructs), this::assertCompileSucceeded, generate);
     }
 
     protected void assertOKWithWarning(String warning, String... constructs) {
-        assertCompile(expandMarkers(constructs), () -> assertCompileSucceededWithWarning(warning));
+        assertCompile(expandMarkers(constructs), () -> assertCompileSucceededWithWarning(warning), false);
     }
 
     protected void assertFail(String expectedDiag, String... constructs) {
-        assertCompile(expandMarkers(constructs), () -> assertCompileFailed(expectedDiag));
+        assertCompile(expandMarkers(constructs), () -> assertCompileFailed(expectedDiag), false);
     }
 }
--- a/test/langtools/tools/javac/records/RecordCompilationTests.java	Tue Jan 14 23:40:42 2020 +0100
+++ b/test/langtools/tools/javac/records/RecordCompilationTests.java	Tue Jan 14 21:49:43 2020 -0500
@@ -23,6 +23,21 @@
  * questions.
  */
 
+/**
+ * RecordCompilationTests
+ *
+ * @test
+ * @summary Negative compilation tests, and positive compilation (smoke) tests for records
+ * @library /lib/combo
+ * @modules
+ *      jdk.compiler/com.sun.tools.javac.util
+ *      jdk.jdeps/com.sun.tools.classfile
+ * @compile --enable-preview -source ${jdk.version} RecordCompilationTests.java
+ * @run testng/othervm --enable-preview RecordCompilationTests
+ */
+
+import java.io.File;
+
 import java.lang.annotation.ElementType;
 import java.util.EnumMap;
 import java.util.EnumSet;
@@ -30,22 +45,18 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import com.sun.tools.javac.util.Assert;
+
+import com.sun.tools.classfile.ClassFile;
+import com.sun.tools.classfile.ConstantPool;
+import com.sun.tools.classfile.ConstantPool.CPInfo;
+
 import org.testng.annotations.Test;
 import tools.javac.combo.CompilationTestCase;
 
 import static java.lang.annotation.ElementType.*;
 import static org.testng.Assert.assertEquals;
 
-/**
- * RecordCompilationTests
- *
- * @test
- * @summary Negative compilation tests, and positive compilation (smoke) tests for records
- * @library /lib/combo
- * @modules jdk.compiler/com.sun.tools.javac.util
- * @compile --enable-preview -source ${jdk.version} RecordCompilationTests.java
- * @run testng/othervm --enable-preview RecordCompilationTests
- */
 @Test
 public class RecordCompilationTests extends CompilationTestCase {
 
@@ -520,4 +531,24 @@
                 }
                 """);
     }
+
+    public void testOnlyOneFieldRef() throws Exception {
+        int numberOfFieldRefs = 0;
+        File dir = assertOK(true, "record R(int recordComponent) {}");
+        for (final File fileEntry : dir.listFiles()) {
+            if (fileEntry.getName().equals("R.class")) {
+                ClassFile classFile = ClassFile.read(fileEntry);
+                for (CPInfo cpInfo : classFile.constant_pool.entries()) {
+                    if (cpInfo instanceof ConstantPool.CONSTANT_Fieldref_info) {
+                        numberOfFieldRefs++;
+                        ConstantPool.CONSTANT_NameAndType_info nameAndType =
+                                (ConstantPool.CONSTANT_NameAndType_info)classFile.constant_pool
+                                        .get(((ConstantPool.CONSTANT_Fieldref_info)cpInfo).name_and_type_index);
+                        Assert.check(nameAndType.getName().equals("recordComponent"));
+                    }
+                }
+            }
+        }
+        Assert.check(numberOfFieldRefs == 1);
+    }
 }