changeset 31200:1212849c3200

8117883: nasgen prototype, instance member count calculation is wrong Reviewed-by: hannesw, lagergren
author sundar
date Thu, 18 Jun 2015 19:20:53 +0530
parents 17932ffc49b5
children 271525e41254
files nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/Main.java nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfo.java nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfoCollector.java nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java
diffstat 5 files changed, 87 insertions(+), 43 deletions(-) [+]
line wrap: on
line diff
--- a/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java	Wed Jun 17 13:56:53 2015 +0200
+++ b/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java	Thu Jun 18 19:20:53 2015 +0530
@@ -152,6 +152,7 @@
         }
 
         if (constructor != null) {
+            initPrototype(mi);
             final int arity = constructor.getArity();
             if (arity != MemberInfo.DEFAULT_ARITY) {
                 mi.loadThis();
@@ -193,6 +194,7 @@
     }
 
     private void initFunctionFields(final MethodGenerator mi) {
+        assert memberCount > 0;
         for (final MemberInfo memInfo : scriptClassInfo.getMembers()) {
             if (!memInfo.isConstructorFunction()) {
                 continue;
@@ -204,37 +206,39 @@
     }
 
     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);
     }
 
     /**
--- a/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/Main.java	Wed Jun 17 13:56:53 2015 +0200
+++ b/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/Main.java	Thu Jun 18 19:20:53 2015 +0530
@@ -140,7 +140,7 @@
                 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();
@@ -152,7 +152,7 @@
                     }
                 }
 
-                if (sci.getConstructorMemberCount() > 0 || sci.getConstructor() != null) {
+                if (sci.isConstructorNeeded()) {
                     // generate constructor class
                     final ConstructorGenerator consGen = new ConstructorGenerator(sci);
                     buf = consGen.getClassBytes();
--- a/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfo.java	Wed Jun 17 13:56:53 2015 +0200
+++ b/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfo.java	Thu Jun 18 19:20:53 2015 +0530
@@ -126,10 +126,42 @@
         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 @@
     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 @@
     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++;
             }
--- a/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfoCollector.java	Wed Jun 17 13:56:53 2015 +0200
+++ b/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfoCollector.java	Thu Jun 18 19:20:53 2015 +0530
@@ -288,9 +288,7 @@
                                         where = Where.PROTOTYPE;
                                         break;
                                     case SPECIALIZED_FUNCTION:
-                                        if (isSpecializedConstructor) {
-                                            where = Where.CONSTRUCTOR;
-                                        }
+                                        where = isSpecializedConstructor? Where.CONSTRUCTOR : Where.PROTOTYPE;
                                         //fallthru
                                     default:
                                         break;
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java	Wed Jun 17 13:56:53 2015 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java	Thu Jun 18 19:20:53 2015 +0530
@@ -393,14 +393,6 @@
     }
 
     /**
-     * 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