8117883: nasgen prototype, instance member count calculation is wrong

Reviewed-by: hannesw, lagergren

5 files changed, 87 insertions, 43 deletions

diff --git a/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java b/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java
index 2661e096..97a4e0d8 100644
--- a/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java
+++ b/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java
@@ -152,6 +152,7 @@ public class ConstructorGenerator extends ClassGenerator {
         }
 
         if (constructor != null) {
+            initPrototype(mi);
             final int arity = constructor.getArity();
             if (arity != MemberInfo.DEFAULT_ARITY) {
                 mi.loadThis();
@@ -193,6 +194,7 @@ public class ConstructorGenerator extends ClassGenerator {
     }
 
     private void initFunctionFields(final MethodGenerator mi) {
+        assert memberCount > 0;
         for (final MemberInfo memInfo : scriptClassInfo.getMembers()) {
             if (!memInfo.isConstructorFunction()) {
                 continue;
@@ -204,37 +206,39 @@ public class ConstructorGenerator extends ClassGenerator {
     }
 
     private void initDataFields(final MethodGenerator mi) {
-         for (final MemberInfo memInfo : scriptClassInfo.getMembers()) {
-            if (!memInfo.isConstructorProperty() || memInfo.isFinal()) {
-                continue;
-            }
-            final Object value = memInfo.getValue();
-            if (value != null) {
-                mi.loadThis();
-                mi.loadLiteral(value);
-                mi.putField(className, memInfo.getJavaName(), memInfo.getJavaDesc());
-            } else if (!memInfo.getInitClass().isEmpty()) {
-                final String clazz = memInfo.getInitClass();
-                mi.loadThis();
-                mi.newObject(clazz);
-                mi.dup();
-                mi.invokeSpecial(clazz, INIT, DEFAULT_INIT_DESC);
-                mi.putField(className, memInfo.getJavaName(), memInfo.getJavaDesc());
-            }
+        assert memberCount > 0;
+        for (final MemberInfo memInfo : scriptClassInfo.getMembers()) {
+           if (!memInfo.isConstructorProperty() || memInfo.isFinal()) {
+               continue;
+           }
+           final Object value = memInfo.getValue();
+           if (value != null) {
+               mi.loadThis();
+               mi.loadLiteral(value);
+               mi.putField(className, memInfo.getJavaName(), memInfo.getJavaDesc());
+           } else if (!memInfo.getInitClass().isEmpty()) {
+               final String clazz = memInfo.getInitClass();
+               mi.loadThis();
+               mi.newObject(clazz);
+               mi.dup();
+               mi.invokeSpecial(clazz, INIT, DEFAULT_INIT_DESC);
+               mi.putField(className, memInfo.getJavaName(), memInfo.getJavaDesc());
+           }
         }
+    }
 
-        if (constructor != null) {
-            mi.loadThis();
-            final String protoName = scriptClassInfo.getPrototypeClassName();
-            mi.newObject(protoName);
-            mi.dup();
-            mi.invokeSpecial(protoName, INIT, DEFAULT_INIT_DESC);
-            mi.dup();
-            mi.loadThis();
-            mi.invokeStatic(PROTOTYPEOBJECT_TYPE, PROTOTYPEOBJECT_SETCONSTRUCTOR,
-                    PROTOTYPEOBJECT_SETCONSTRUCTOR_DESC);
-            mi.invokeVirtual(SCRIPTFUNCTION_TYPE, SCRIPTFUNCTION_SETPROTOTYPE, SCRIPTFUNCTION_SETPROTOTYPE_DESC);
-        }
+    private void initPrototype(final MethodGenerator mi) {
+        assert constructor != null;
+        mi.loadThis();
+        final String protoName = scriptClassInfo.getPrototypeClassName();
+        mi.newObject(protoName);
+        mi.dup();
+        mi.invokeSpecial(protoName, INIT, DEFAULT_INIT_DESC);
+        mi.dup();
+        mi.loadThis();
+        mi.invokeStatic(PROTOTYPEOBJECT_TYPE, PROTOTYPEOBJECT_SETCONSTRUCTOR,
+                PROTOTYPEOBJECT_SETCONSTRUCTOR_DESC);
+        mi.invokeVirtual(SCRIPTFUNCTION_TYPE, SCRIPTFUNCTION_SETPROTOTYPE, SCRIPTFUNCTION_SETPROTOTYPE_DESC);
     }
 
     /**
diff --git a/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/Main.java b/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/Main.java
index 7c8439b3..554c09ed 100644
--- a/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/Main.java
+++ b/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/Main.java
@@ -134,7 +134,7 @@ public class Main {
                 String simpleName = inFile.getName();
                 simpleName = simpleName.substring(0, simpleName.indexOf(".class"));
 
-                if (sci.getPrototypeMemberCount() > 0) {
+                if (sci.isPrototypeNeeded()) {
                     // generate prototype class
                     final PrototypeGenerator protGen = new PrototypeGenerator(sci);
                     buf = protGen.getClassBytes();
@@ -146,7 +146,7 @@ public class Main {
                     }
                 }
 
-                if (sci.getConstructorMemberCount() > 0 || sci.getConstructor() != null) {
+                if (sci.isConstructorNeeded()) {
                     // generate constructor class
                     final ConstructorGenerator consGen = new ConstructorGenerator(sci);
                     buf = consGen.getClassBytes();
diff --git a/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfo.java b/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfo.java
index 9e3dfc17..c91d7585 100644
--- a/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfo.java
+++ b/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfo.java
@@ -126,10 +126,42 @@ public final class ScriptClassInfo {
         return Collections.unmodifiableList(res);
     }
 
+    boolean isConstructorNeeded() {
+        // Constructor class generation is needed if we one or
+        // more constructor properties are defined or @Constructor
+        // is defined in the class.
+        for (final MemberInfo memInfo : members) {
+            if (memInfo.getKind() == Kind.CONSTRUCTOR ||
+                memInfo.getWhere() == Where.CONSTRUCTOR) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    boolean isPrototypeNeeded() {
+        // Prototype class generation is needed if we have atleast one
+        // prototype property or @Constructor defined in the class.
+        for (final MemberInfo memInfo : members) {
+            if (memInfo.getWhere() == Where.PROTOTYPE || memInfo.isConstructor()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     int getPrototypeMemberCount() {
         int count = 0;
         for (final MemberInfo memInfo : members) {
-            if (memInfo.getWhere() == Where.PROTOTYPE || memInfo.isConstructor()) {
+            switch (memInfo.getKind()) {
+                case SETTER:
+                case SPECIALIZED_FUNCTION:
+                    // SETTER was counted when GETTER was encountered.
+                    // SPECIALIZED_FUNCTION was counted as FUNCTION already.
+                    continue;
+            }
+
+            if (memInfo.getWhere() == Where.PROTOTYPE) {
                 count++;
             }
         }
@@ -139,6 +171,16 @@ public final class ScriptClassInfo {
     int getConstructorMemberCount() {
         int count = 0;
         for (final MemberInfo memInfo : members) {
+            switch (memInfo.getKind()) {
+                case CONSTRUCTOR:
+                case SETTER:
+                case SPECIALIZED_FUNCTION:
+                    // SETTER was counted when GETTER was encountered.
+                    // Constructor and constructor SpecializedFunctions
+                    // are not added as members and so not counted.
+                    continue;
+            }
+
             if (memInfo.getWhere() == Where.CONSTRUCTOR) {
                 count++;
             }
@@ -149,6 +191,14 @@ public final class ScriptClassInfo {
     int getInstancePropertyCount() {
         int count = 0;
         for (final MemberInfo memInfo : members) {
+            switch (memInfo.getKind()) {
+                case SETTER:
+                case SPECIALIZED_FUNCTION:
+                    // SETTER was counted when GETTER was encountered.
+                    // SPECIALIZED_FUNCTION was counted as FUNCTION already.
+                    continue;
+            }
+
             if (memInfo.getWhere() == Where.INSTANCE) {
                 count++;
             }
diff --git a/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfoCollector.java b/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfoCollector.java
index aa477621..ae58aadd 100644
--- a/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfoCollector.java
+++ b/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfoCollector.java
@@ -288,9 +288,7 @@ public class ScriptClassInfoCollector extends ClassVisitor {
                                         where = Where.PROTOTYPE;
                                         break;
                                     case SPECIALIZED_FUNCTION:
-                                        if (isSpecializedConstructor) {
-                                            where = Where.CONSTRUCTOR;
-                                        }
+                                        where = isSpecializedConstructor? Where.CONSTRUCTOR : Where.PROTOTYPE;
                                         //fallthru
                                     default:
                                         break;
diff --git a/src/jdk/nashorn/internal/runtime/ScriptObject.java b/src/jdk/nashorn/internal/runtime/ScriptObject.java
index 881fb995..e041e517 100644
--- a/src/jdk/nashorn/internal/runtime/ScriptObject.java
+++ b/src/jdk/nashorn/internal/runtime/ScriptObject.java
@@ -393,14 +393,6 @@ public abstract class ScriptObject implements PropertyAccess, Cloneable {
     }
 
     /**
-     * ECMA 8.10.3 IsGenericDescriptor ( Desc )
-     * @return true if this has a descriptor describing an {@link AccessorPropertyDescriptor} or {@link DataPropertyDescriptor}
-     */
-    public final boolean isGenericDescriptor() {
-        return isAccessorDescriptor() || isDataDescriptor();
-    }
-
-    /**
       * ECMA 8.10.5 ToPropertyDescriptor ( Obj )
       *
       * @return property descriptor