From 50b0b601b5e39f0da7040fbd0a868d75b6e6f363 Mon Sep 17 00:00:00 2001 From: vromero Date: Sat, 1 Jun 2013 21:57:56 +0100 Subject: [PATCH] 8010737: javac, known parameter's names should be copied to automatically generated constructors for inner classes Reviewed-by: mcimadamore --- .../com/sun/tools/javac/comp/MemberEnter.java | 78 ++++-- .../MethodParameters/ClassFileVisitor.java | 3 - .../MethodParameters/ReflectionVisitor.java | 3 - ...rNamesAreNotCopiedToAnonymousInitTest.java | 239 ++++++++++++++++++ 4 files changed, 302 insertions(+), 21 deletions(-) create mode 100644 test/tools/javac/T8010737/ParameterNamesAreNotCopiedToAnonymousInitTest.java diff --git a/src/share/classes/com/sun/tools/javac/comp/MemberEnter.java b/src/share/classes/com/sun/tools/javac/comp/MemberEnter.java index 74e3d97b..62c3fa02 100644 --- a/src/share/classes/com/sun/tools/javac/comp/MemberEnter.java +++ b/src/share/classes/com/sun/tools/javac/comp/MemberEnter.java @@ -996,8 +996,9 @@ public class MemberEnter extends JCTree.Visitor implements Completer { long ctorFlags = 0; boolean based = false; boolean addConstructor = true; + JCNewClass nc = null; if (c.name.isEmpty()) { - JCNewClass nc = (JCNewClass)env.next.tree; + nc = (JCNewClass)env.next.tree; if (nc.constructor != null) { addConstructor = nc.constructor.kind != ERR; Type superConstrType = types.memberType(c.type, @@ -1013,7 +1014,10 @@ public class MemberEnter extends JCTree.Visitor implements Completer { } } if (addConstructor) { + MethodSymbol basedConstructor = nc != null ? + (MethodSymbol)nc.constructor : null; JCTree constrDef = DefaultConstructor(make.at(tree.pos), c, + basedConstructor, typarams, argtypes, thrown, ctorFlags, based); tree.defs = tree.defs.prepend(constrDef); @@ -1399,34 +1403,78 @@ public class MemberEnter extends JCTree.Visitor implements Completer { */ JCTree DefaultConstructor(TreeMaker make, ClassSymbol c, + MethodSymbol baseInit, List typarams, List argtypes, List thrown, long flags, boolean based) { - List params = make.Params(argtypes, syms.noSymbol); - List stats = List.nil(); - if (c.type != syms.objectType) - stats = stats.prepend(SuperCall(make, typarams, params, based)); + JCTree result; if ((c.flags() & ENUM) != 0 && (types.supertype(c.type).tsym == syms.enumSym)) { // constructors of true enums are private flags = (flags & ~AccessFlags) | PRIVATE | GENERATEDCONSTR; } else flags |= (c.flags() & AccessFlags) | GENERATEDCONSTR; - if (c.name.isEmpty()) flags |= ANONCONSTR; - JCTree result = make.MethodDef( - make.Modifiers(flags), - names.init, - null, - make.TypeParams(typarams), - params, - make.Types(thrown), - make.Block(0, stats), - null); + if (c.name.isEmpty()) { + flags |= ANONCONSTR; + } + Type mType = new MethodType(argtypes, null, thrown, c); + Type initType = typarams.nonEmpty() ? + new ForAll(typarams, mType) : + mType; + MethodSymbol init = new MethodSymbol(flags, names.init, + initType, c); + init.params = createDefaultConstructorParams(make, baseInit, init, + argtypes, based); + List params = make.Params(argtypes, init); + List stats = List.nil(); + if (c.type != syms.objectType) { + stats = stats.prepend(SuperCall(make, typarams, params, based)); + } + result = make.MethodDef(init, make.Block(0, stats)); return result; } + private List createDefaultConstructorParams( + TreeMaker make, + MethodSymbol baseInit, + MethodSymbol init, + List argtypes, + boolean based) { + List initParams = null; + List argTypesList = argtypes; + if (based) { + /* In this case argtypes will have an extra type, compared to baseInit, + * corresponding to the type of the enclosing instance i.e.: + * + * Inner i = outer.new Inner(1){} + * + * in the above example argtypes will be (Outer, int) and baseInit + * will have parameter's types (int). So in this case we have to add + * first the extra type in argtypes and then get the names of the + * parameters from baseInit. + */ + initParams = List.nil(); + VarSymbol param = new VarSymbol(0, make.paramName(0), argtypes.head, init); + initParams = initParams.append(param); + argTypesList = argTypesList.tail; + } + if (baseInit != null && baseInit.params != null && + baseInit.params.nonEmpty() && argTypesList.nonEmpty()) { + initParams = (initParams == null) ? List.nil() : initParams; + List baseInitParams = baseInit.params; + while (baseInitParams.nonEmpty() && argTypesList.nonEmpty()) { + VarSymbol param = new VarSymbol(baseInitParams.head.flags(), + baseInitParams.head.name, argTypesList.head, init); + initParams = initParams.append(param); + baseInitParams = baseInitParams.tail; + argTypesList = argTypesList.tail; + } + } + return initParams; + } + /** Generate call to superclass constructor. This is: * * super(id_0, ..., id_n) diff --git a/test/tools/javac/MethodParameters/ClassFileVisitor.java b/test/tools/javac/MethodParameters/ClassFileVisitor.java index 44daed1f..c5d1d93c 100644 --- a/test/tools/javac/MethodParameters/ClassFileVisitor.java +++ b/test/tools/javac/MethodParameters/ClassFileVisitor.java @@ -316,9 +316,6 @@ class ClassFileVisitor extends Tester.Visitor { } expect = "this\\$[0-n]*"; } - } else if (isAnon) { - // not an implementation gurantee, but okay for now - expect = "x[0-n]*"; } } else if (isEnum && mNumParams == 1 && index == 0 && mName.equals("valueOf")) { expect = "name"; diff --git a/test/tools/javac/MethodParameters/ReflectionVisitor.java b/test/tools/javac/MethodParameters/ReflectionVisitor.java index e1b1a32c..a7129ba9 100644 --- a/test/tools/javac/MethodParameters/ReflectionVisitor.java +++ b/test/tools/javac/MethodParameters/ReflectionVisitor.java @@ -151,9 +151,6 @@ public class ReflectionVisitor extends Tester.Visitor { } expect = "this\\$[0-n]*"; } - } else if (isAnon) { - // not an implementation gurantee, but okay for now - expect = "x[0-n]*"; } // Check expected flags diff --git a/test/tools/javac/T8010737/ParameterNamesAreNotCopiedToAnonymousInitTest.java b/test/tools/javac/T8010737/ParameterNamesAreNotCopiedToAnonymousInitTest.java new file mode 100644 index 00000000..f58d186f --- /dev/null +++ b/test/tools/javac/T8010737/ParameterNamesAreNotCopiedToAnonymousInitTest.java @@ -0,0 +1,239 @@ +/* + * Copyright (c) 2013, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8010737 + * @summary javac, known parameter's names should be copied to automatically + * generated constructors for inner classes + * @run main ParameterNamesAreNotCopiedToAnonymousInitTest check_class_file check_init_symbol + */ + +import java.io.File; +import java.io.IOException; +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; +import java.nio.file.Paths; +import java.util.Arrays; + +import javax.tools.JavaCompiler; +import javax.tools.JavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.ToolProvider; + +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.Tree; +import com.sun.source.util.JavacTask; +import com.sun.source.util.TaskEvent; +import com.sun.source.util.TaskListener; +import com.sun.tools.classfile.ClassFile; +import com.sun.tools.classfile.Method; +import com.sun.tools.javac.api.BasicJavacTask; +import com.sun.tools.javac.code.Attribute.Compound; +import com.sun.tools.javac.code.Symbol.ClassSymbol; +import com.sun.tools.javac.code.Symbol.VarSymbol; +import com.sun.tools.javac.tree.JCTree; +import com.sun.tools.javac.tree.TreeScanner; +import com.sun.tools.javac.util.Assert; +import com.sun.tools.javac.util.Context; +import com.sun.tools.javac.util.List; +import com.sun.tools.javac.util.Names; + +public class ParameterNamesAreNotCopiedToAnonymousInitTest { + + static final String noParamsErrorMsg = + "Test most be invoked with at least one parameter: check_class_file " + + "and/or check_init_symbol"; + static final String wrongParamsErrorMsg = + "Accepted arguments are: check_class_file and check_init_symbol"; + static final String paramNameNotCopiedAssertionMsg = + "The param name hasn't been copied to the init method"; + static final String noAnnotationsForParameterMsg = + "No annotations for seek parameter"; + static final String seekMethodNotFound = + "The seek init method was not found or conditions were not met"; + static final String nonNullParamPositionsMsg = + "Parameter positions shold not be null"; + static final String compilationFailed = + "Compilation failed"; + static final String seekMethodNotFoundMsg = + "The seek method was not found"; + + static final String ParamAnnotationClassName = + ParameterNamesAreNotCopiedToAnonymousInitTest.class.getSimpleName() + "." + + ParamAnnotation.class.getSimpleName(); + + public static void main(String[] args) throws Exception { + if (args.length == 0) { + throw new Error(noParamsErrorMsg); + } + new ParameterNamesAreNotCopiedToAnonymousInitTest().run(args); + } + + void run(String[] args) throws Exception { + for (String arg : args) { + if (arg.equals("check_class_file")) { + checkClassFile(new File(Paths.get(System.getProperty("test.classes"), + this.getClass().getName() + "$initParams$1.class").toUri()), 1); + checkClassFile(new File(Paths.get(System.getProperty("test.classes"), + this.getClass().getName() + "$Generics$1.class").toUri()), 2); + } else if (arg.equals("check_init_symbol")) { + checkInitSymbol("m1", Arrays.asList(0), Arrays.asList("i")); + checkInitSymbol("m2", Arrays.asList(0, 1), Arrays.asList("t1", "t2")); + } else { + error(wrongParamsErrorMsg); + } + } + } + + void checkClassFile(final File cfile, int numberOfParams) throws Exception { + ClassFile classFile = ClassFile.read(cfile); + boolean methodFound = false; + for (Method method : classFile.methods) { + if (method.getName(classFile.constant_pool).equals("")) { + methodFound = true; + } + } + Assert.check(methodFound, seekMethodNotFoundMsg); + } + + /* This method expect a non-null ordered list of integers, listing the + * position of the parameters to be checked on the init method. Position 0 + * corresponds to the first parameter. + * + * As we are looking for a constructor of an anonymous class, the + * classOwnerName parameter must be the name of the method where the + * anonymous class is declared. + */ + void checkInitSymbol( + final String classOwnerName, + final java.util.List paramsToCheck, + final java.util.List paramNames) + throws IOException { + Assert.checkNonNull(paramsToCheck, nonNullParamPositionsMsg); + JavaCompiler c = ToolProvider.getSystemJavaCompiler(); + StandardJavaFileManager fm = c.getStandardFileManager(null, null, null); + Iterable fos = + fm.getJavaFileObjectsFromFiles( + Arrays.asList(new File(System.getProperty("test.src"), + this.getClass().getName() + ".java"))); + JavacTask task = (JavacTask) c.getTask(null, fm, null, + Arrays.asList("-d", System.getProperty("user.dir")), null, fos); + + BasicJavacTask impl = (BasicJavacTask)task; + Context context = impl.getContext(); + final Names names = Names.instance(context); + + task.addTaskListener(new TaskListener() { + + @Override + public void started(TaskEvent e) {} + + @Override + public void finished(TaskEvent e) { + class TheTreeScanner extends TreeScanner { + boolean foundAndCorrect = false; + + @Override + public void visitMethodDef(JCTree.JCMethodDecl tree) { + ClassSymbol clazz = (ClassSymbol)tree.sym.owner; + if (clazz.owner.name.toString().equals(classOwnerName) && + tree.sym.name == names.init) { + + int currentParamPos = 0; + int paramArrayIndex = 0; + + List params = tree.sym.params; + while (params.nonEmpty() && paramArrayIndex < paramsToCheck.size()) { + VarSymbol param = params.head; + if (currentParamPos == paramsToCheck.get(paramArrayIndex)) { + if (!param.name.toString() + .equals(paramNames.get(paramArrayIndex))) { + error(paramNameNotCopiedAssertionMsg); + } + paramArrayIndex++; + } + currentParamPos++; + params = params.tail; + } + foundAndCorrect = paramArrayIndex >= paramsToCheck.size(); + } + super.visitMethodDef(tree); + } + } + + if (e.getKind() == TaskEvent.Kind.ANALYZE) { + CompilationUnitTree compUnitTree = e.getCompilationUnit(); + boolean foundAndCorrect = false; + for (Tree tree : compUnitTree.getTypeDecls()) { + TheTreeScanner scanner = new TheTreeScanner(); + scanner.scan((JCTree) tree); + foundAndCorrect = foundAndCorrect | scanner.foundAndCorrect; + } + if (!foundAndCorrect) { + error(seekMethodNotFound); + } + } + } + }); + + if (!task.call()) { + error(compilationFailed); + } + } + + void error(String msg) { + throw new AssertionError(msg); + } + + @Target(value = {ElementType.PARAMETER}) + @interface ParamAnnotation {} + + /* If more cases are added in the future, it should be taken into account + * that method checkInitSymbol locates the inner class looking for its + * container method, which in the cases below are m1 and m2. So new cases + * must have different names for container methods or method checkInitSymbol + * should be changed. + */ + public class initParams { + public initParams(@ParamAnnotation int i) {} + + public void m1() { + new initParams(2) {}; + } + } + + class Generics { + T1 obj1; + Object obj2; + Generics(@ParamAnnotation T1 t1, @ParamAnnotation T2 t2) { + obj1 = t1; + obj2 = t2; + } + + void m2() { + Generics a = new Generics( + new Integer(11), "foo") {}; + } + } +} -- GitLab