# HG changeset patch
# Parent 78b6cac476d9ce01916f23f7a87d0d3083bb8be2
8007720: Names are not load correctly for method parameters if the parameters have annotations
Reviewed-by: TBD

diff -r 78b6cac476d9 src/share/classes/com/sun/tools/javac/code/Symbol.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java	Tue Jan 21 09:25:10 2014 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java	Wed Jan 22 16:00:26 2014 +0100
@@ -1290,9 +1290,6 @@
         /** The parameters of the method. */
         public List<VarSymbol> params = null;
 
-        /** The names of the parameters */
-        public List<Name> savedParameterNames;
-
         /** For an attribute field accessor, its default value if any.
          *  The value is null if none appeared in the method
          *  declaration.
@@ -1528,59 +1525,19 @@
         public List<VarSymbol> params() {
             owner.complete();
             if (params == null) {
-                // If ClassReader.saveParameterNames has been set true, then
-                // savedParameterNames will be set to a list of names that
-                // matches the types in type.getParameterTypes().  If any names
-                // were not found in the class file, those names in the list will
-                // be set to the empty name.
-                // If ClassReader.saveParameterNames has been set false, then
-                // savedParameterNames will be null.
-                List<Name> paramNames = savedParameterNames;
-                savedParameterNames = null;
-                // discard the provided names if the list of names is the wrong size.
-                if (paramNames == null || paramNames.size() != type.getParameterTypes().size()) {
-                    paramNames = List.nil();
-                }
-                ListBuffer<VarSymbol> buf = new ListBuffer<>();
-                List<Name> remaining = paramNames;
-                // assert: remaining and paramNames are both empty or both
-                // have same cardinality as type.getParameterTypes()
+                ListBuffer<VarSymbol> newParams = new ListBuffer<>();
                 int i = 0;
                 for (Type t : type.getParameterTypes()) {
-                    Name paramName;
-                    if (remaining.isEmpty()) {
-                        // no names for any parameters available
-                        paramName = createArgName(i, paramNames);
-                    } else {
-                        paramName = remaining.head;
-                        remaining = remaining.tail;
-                        if (paramName.isEmpty()) {
-                            // no name for this specific parameter
-                            paramName = createArgName(i, paramNames);
-                        }
-                    }
-                    buf.append(new VarSymbol(PARAMETER, paramName, t, this));
-                    i++;
+                    Name paramName = name.table.fromString("arg" + i);
+                    VarSymbol param = new VarSymbol(PARAMETER, paramName, t, this);
+                    newParams.append(param);
                 }
-                params = buf.toList();
+                params = newParams.toList();
             }
+            Assert.checkNonNull(params);
             return params;
         }
 
-        // Create a name for the argument at position 'index' that is not in
-        // the exclude list. In normal use, either no names will have been
-        // provided, in which case the exclude list is empty, or all the names
-        // will have been provided, in which case this method will not be called.
-        private Name createArgName(int index, List<Name> exclude) {
-            String prefix = "arg";
-            while (true) {
-                Name argName = name.table.fromString(prefix + index);
-                if (!exclude.contains(argName))
-                    return argName;
-                prefix += "$";
-            }
-        }
-
         public Symbol asMemberOf(Type site, Types types) {
             return new MethodSymbol(flags_field, name, types.memberType(site, this), owner);
         }
diff -r 78b6cac476d9 src/share/classes/com/sun/tools/javac/jvm/ClassReader.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java	Tue Jan 21 09:25:10 2014 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java	Wed Jan 22 16:00:26 2014 +0100
@@ -213,8 +213,9 @@
      */
     int[] parameterNameIndices;
 
+    List<CompoundAnnotationProxy>[] parameterAnnotations;
+    
     /**
-     * Whether or not any parameter names have been found.
      */
     boolean haveParameterNameIndices;
 
@@ -1159,7 +1160,7 @@
 
             new AttributeReader(names.RuntimeInvisibleParameterAnnotations, V49, CLASS_OR_MEMBER_ATTRIBUTE) {
                 protected void read(Symbol sym, int attrLen) {
-                    attachParameterAnnotations(sym);
+                    readParameterAnnotations(sym);
                 }
             },
 
@@ -1171,7 +1172,7 @@
 
             new AttributeReader(names.RuntimeVisibleParameterAnnotations, V49, CLASS_OR_MEMBER_ATTRIBUTE) {
                 protected void read(Symbol sym, int attrLen) {
-                    attachParameterAnnotations(sym);
+                    readParameterAnnotations(sym);
                 }
             },
 
@@ -1420,21 +1421,26 @@
         }
     }
 
-    /** Attach parameter annotations.
+    /** Read parameter annotations.
      */
-    void attachParameterAnnotations(final Symbol method) {
-        final MethodSymbol meth = (MethodSymbol)method;
+    @SuppressWarnings({"rawtypes", "unchecked"})
+    void readParameterAnnotations(final Symbol method) {
         int numParameters = buf[bp++] & 0xFF;
-        List<VarSymbol> parameters = meth.params();
-        int pnum = 0;
-        while (parameters.tail != null) {
-            attachAnnotations(parameters.head);
-            parameters = parameters.tail;
-            pnum++;
+        if (parameterAnnotations == null)
+            parameterAnnotations = new List[numParameters];
+        for (int pnum = 0; pnum < numParameters; pnum++) {
+            int numAttributes = nextChar();
+            if (numAttributes != 0) {
+                ListBuffer<CompoundAnnotationProxy> proxies = new ListBuffer<>();
+                for (int i = 0; i<numAttributes; i++) {
+                    proxies.append(readCompoundAnnotation());
+                }
+                parameterAnnotations[pnum] = parameterAnnotations[pnum] != null ? parameterAnnotations[pnum].prependList(proxies.toList()) : proxies.toList();
+            }
         }
-        if (pnum != numParameters) {
-            throw badClassFile("bad.runtime.invisible.param.annotations", meth);
-        }
+//        if (pnum != numParameters) {
+//            throw badClassFile("bad.runtime.invisible.param.annotations", meth);
+//        }
     }
 
     void attachTypeAnnotations(final Symbol sym) {
@@ -2017,8 +2023,7 @@
         } finally {
             currentOwner = prevOwner;
         }
-        if (saveParameterNames)
-            setParameterNames(m, type);
+        setParameterNames(m, type);
         return m;
     }
 
@@ -2075,19 +2080,20 @@
      * any missing names represented by the empty name.
      */
     void setParameterNames(MethodSymbol sym, Type jvmType) {
-        // if no names were found in the class file, there's nothing more to do
-        if (!haveParameterNameIndices)
-            return;
+//        // if no names were found in the class file, there's nothing more to do
+//        if (!haveParameterNameIndices)
+//            return;
         // If we get parameter names from MethodParameters, then we
         // don't need to skip.
         int firstParam = 0;
         if (!sawMethodParameters) {
             firstParam = ((sym.flags() & STATIC) == 0) ? 1 : 0;
-            // the code in readMethod may have skipped the first
-            // parameter when setting up the MethodType. If so, we
-            // make a corresponding allowance here for the position of
-            // the first parameter.  Note that this assumes the
-            // skipped parameter has a width of 1 -- i.e. it is not
+        }
+        // the code in readMethod may have skipped the first
+        // parameter when setting up the MethodType. If so, we
+        // make a corresponding allowance here for the position of
+        // the first parameter.  Note that this assumes the
+        // skipped parameter has a width of 1 -- i.e. it is not
         // a double width type (long or double.)
         if (sym.name == names.init && currentOwner.hasOuterInstance()) {
             // Sometimes anonymous classes don't have an outer
@@ -2098,30 +2104,52 @@
         }
 
         if (sym.type != jvmType) {
-                // reading the method attributes has caused the
-                // symbol's type to be changed. (i.e. the Signature
-                // attribute.)  This may happen if there are hidden
-                // (synthetic) parameters in the descriptor, but not
-                // in the Signature.  The position of these hidden
-                // parameters is unspecified; for now, assume they are
-                // at the beginning, and so skip over them. The
-                // primary case for this is two hidden parameters
-                // passed into Enum constructors.
+            // reading the method attributes has caused the
+            // symbol's type to be changed. (i.e. the Signature
+            // attribute.)  This may happen if there are hidden
+            // (synthetic) parameters in the descriptor, but not
+            // in the Signature.  The position of these hidden
+            // parameters is unspecified; for now, assume they are
+            // at the beginning, and so skip over them. The
+            // primary case for this is two hidden parameters
+            // passed into Enum constructors.
             int skip = Code.width(jvmType.getParameterTypes())
                     - Code.width(sym.type.getParameterTypes());
             firstParam += skip;
         }
-        }
         List<Name> paramNames = List.nil();
+        ListBuffer<VarSymbol> params = new ListBuffer<>();
         int index = firstParam;
-        for (Type t: sym.type.getParameterTypes()) {
-            int nameIdx = (index < parameterNameIndices.length
+        int i = 0;
+        for (Type t : sym.type.getParameterTypes()) {
+            int nameIdx = (parameterNameIndices != null && index < parameterNameIndices.length
                     ? parameterNameIndices[index] : 0);
-            Name name = nameIdx == 0 ? names.empty : readName(nameIdx);
+            Name name = nameIdx == 0 ? createArgName(i, paramNames) : readName(nameIdx);
+            VarSymbol param = new VarSymbol(PARAMETER, name, t, sym);
+            params.append(param);
+            if (parameterAnnotations != null && parameterAnnotations[i] != null && parameterAnnotations[i].nonEmpty()) {
+                annotate.normal(new AnnotationCompleter(param, parameterAnnotations[i]));
+            }
             paramNames = paramNames.prepend(name);
             index += Code.width(t);
+            i++;
         }
-        sym.savedParameterNames = paramNames.reverse();
+        sym.params = params.toList();
+        parameterAnnotations = null;
+    }
+    
+    // Create a name for the argument at position 'index' that is not in
+    // the exclude list. In normal use, either no names will have been
+    // provided, in which case the exclude list is empty, or all the names
+    // will have been provided, in which case this method will not be called.
+    private Name createArgName(int index, List<Name> exclude) {
+        String prefix = "arg";
+        while (true) {
+            Name argName = names.fromString(prefix + index);
+            if (!exclude.contains(argName))
+                return argName;
+            prefix += "$";
+        }
     }
 
     /**
diff -r 78b6cac476d9 test/tools/javac/8007720/ParameterNames.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/tools/javac/8007720/ParameterNames.java	Wed Jan 22 16:00:26 2014 +0100
@@ -0,0 +1,198 @@
+/*
+ * 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 8007720
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import javax.lang.model.SourceVersion;
+import javax.lang.model.element.ExecutableElement;
+import javax.lang.model.element.TypeElement;
+import javax.lang.model.element.VariableElement;
+import javax.lang.model.util.ElementFilter;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
+import javax.tools.SimpleJavaFileObject;
+import javax.tools.ToolProvider;
+import com.sun.source.util.JavacTask;
+import com.sun.tools.classfile.Attribute;
+import com.sun.tools.classfile.Attributes;
+import com.sun.tools.classfile.ClassFile;
+import com.sun.tools.classfile.ClassWriter;
+import com.sun.tools.classfile.ConstantPoolException;
+import com.sun.tools.classfile.Method;
+import com.sun.tools.javac.api.JavacTaskImpl;
+import com.sun.tools.javac.code.Symbol.ClassSymbol;
+import com.sun.tools.javac.code.Symbol.CompletionFailure;
+import com.sun.tools.javac.code.Symtab;
+import com.sun.tools.javac.util.Context;
+import com.sun.tools.javac.util.Names;
+
+public class ParameterNames {
+
+    public static void main(String... args) throws Exception {
+        new ParameterNames().run();
+    }
+    
+    void run() throws Exception {
+        File testClasses = new File(System.getProperty("test.classes"));
+        List<String> attributes = Arrays.asList(Attribute.Code, Attribute.MethodParameters, Attribute.RuntimeVisibleParameterAnnotations, Attribute.Signature);
+
+        for (String keepParameters : new String[] {"-g", "-parameters", null}) {
+            for (List<String> attributesPermutation : permutations(attributes)) {
+                JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
+
+                List<String> generateOptions = new ArrayList<>(Arrays.asList("-d", System.getProperty("test.classes"), keepParameters));
+                generateOptions.remove(null);
+                JavacTask compileTask = (JavacTask) compiler.getTask(null, null, null, generateOptions, null, Arrays.asList(new MyFileObject("package test; public enum Test { A; private Test() { } private Test(@Deprecated byte byteName) { }  private class Inner { public Inner(@Deprecated byte byteName) { } } }")));
+
+                for (JavaFileObject f : compileTask.generate()) {
+                    String name = testClasses.toURI().relativize(f.toUri()).getPath();
+                    name = name.substring(0, name.length() - ".class".length());
+                    reorderAttributes(new File(f.toUri()), attributesPermutation);
+
+                    for (String readParameters : new String[] {"-XDsave-parameter-names", null}) {
+                        List<String> readOptions = new ArrayList<>(Arrays.asList("-classpath", System.getProperty("test.classes"), readParameters));
+                        readOptions.remove(null);
+                        boolean expectParameterNames = keepParameters != null && readParameters != null;
+                        JavacTaskImpl javacTask = (JavacTaskImpl) compiler.getTask(null, null, null, readOptions, null, null);
+
+                        TypeElement typeElement = getTypeElementByBinaryName(javacTask.getContext(), name.replace('/', '.'));
+
+                        for (ExecutableElement method : ElementFilter.constructorsIn(typeElement.getEnclosedElements())) {
+                            if (method.getParameters().isEmpty()) continue;
+
+                            VariableElement params = method.getParameters().get(0);
+                            String expectedName = expectParameterNames ? "byteName" : "arg0";
+
+                            if (!params.getSimpleName().contentEquals(expectedName)) {
+                                throw new IllegalStateException("Unexpected parameter name: " + params.getSimpleName());
+                            }
+                            if (params.getAnnotationMirrors().size() != 1) {
+                                throw new IllegalStateException("Expected some parameter annotations.");
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    private void reorderAttributes(File classFile, List<String> attributesPermutation) throws IOException, ConstantPoolException {
+        ClassFile cf = ClassFile.read(classFile);
+        List<Method> newMethods = new ArrayList<>();
+
+        for (Method m : cf.methods) {
+            List<Attribute> newAttributes = new ArrayList<>();
+
+            for (String attrName : attributesPermutation) {
+                Attribute attr = m.attributes.get(attrName);
+                if (attr != null)
+                    newAttributes.add(attr);
+            }
+
+            for (Attribute attr : m.attributes) {
+                if (!newAttributes.contains(attr)) newAttributes.add(attr);
+            }
+
+            newMethods.add(new Method(m.access_flags, m.name_index, m.descriptor, new Attributes(cf.constant_pool, newAttributes.toArray(new Attribute[0]))));
+        }
+
+        cf = new ClassFile(cf.magic, cf.minor_version, cf.major_version, cf.constant_pool, cf.access_flags, cf.this_class, cf.super_class, cf.interfaces, cf.fields, newMethods.toArray(new Method[0]), cf.attributes);
+
+        new ClassWriter().write(cf, classFile);
+    }
+
+    private static List<List<String>> permutations(List<String> attributes) {
+        if (attributes.isEmpty()) return Collections.singletonList(new ArrayList<>());
+
+        List<List<String>> result = new ArrayList<>();
+
+        for (String attribute : attributes) {
+            List<String> copy = new ArrayList<>(attributes);
+
+            copy.remove(attribute);
+
+            for (List<String> perm : permutations(copy)) {
+                perm.add(attribute);
+                result.add(perm);
+            }
+        }
+
+        return result;
+    }
+
+    static class MyFileObject extends SimpleJavaFileObject {
+        private String text;
+        public MyFileObject(String text) {
+            super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE);
+            this.text = text;
+        }
+        @Override
+        public CharSequence getCharContent(boolean ignoreEncodingErrors) {
+            return text;
+        }
+    }
+
+    public ClassSymbol getTypeElementByBinaryName(Context context, final CharSequence binaryName) {
+        final String strName = binaryName instanceof String ? (String) binaryName : binaryName.toString();
+        int index = strName.indexOf('$');   //NOI18N
+        final String owner = index < 0 ? strName : strName.substring(0,index);
+        return SourceVersion.isName(owner)
+            ? binaryNameToClassSymbol(context, strName, owner)
+            : null;
+    }
+
+    private ClassSymbol binaryNameToClassSymbol(Context context, final String binaryName, final String owner) {
+        final com.sun.tools.javac.util.Name name = Names.instance(context).fromString(binaryName);
+        ClassSymbol sym = Symtab.instance(context).classes.get(name);
+        try {
+            if (sym == null) {
+                com.sun.tools.javac.main.JavaCompiler.instance(context).resolveIdent(owner);
+                sym = Symtab.instance(context).classes.get(name);
+            }
+
+            if (sym != null) {
+                sym.complete();
+                return (sym.exists() &&
+                    name.equals(sym.flatName()))
+                    ? sym
+                    : null;
+            }
+            else if (Symtab.instance(context).classes.get(owner) != null) {
+                throw new AssertionError ("Cannot resolve: " + name + "resolved othermost: " + owner);    //NOI18N
+            }
+        } catch (CompletionFailure e) {
+        }
+        return null;
+    }
+
+}
diff -r 78b6cac476d9 test/tools/javac/8007720/ParameterNames.out
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/tools/javac/8007720/ParameterNames.out	Wed Jan 22 16:00:26 2014 +0100
@@ -0,0 +1,2 @@
+test1(parameterNameForTesting)
+test2(parameterNameForTesting)