Remove unnecessary invalidations in setSuperClass() and cleanup setting the name of modules

* These invalidations had the side effect of computing the name eagerly
  for all classes except BasicObject.

Author: eregon

spec/truffle/interop/meta_object_spec.rb

@@ -41,3 +41,19 @@
     Truffle::Interop.should_not.has_meta_parents?(Enumerable)
   end
 end
+
+describe "Truffle::Interop.meta_object?" do
+  it "returns true for a Class" do
+    Truffle::Interop.should.meta_object?(String)
+    Truffle::Interop.should.meta_object?(Class.new)
+  end
+
+  it "returns true for a Module" do
+    Truffle::Interop.should.meta_object?(Kernel)
+    Truffle::Interop.should.meta_object?(Module.new)
+  end
+
+  it "returns false for objects" do
+    Truffle::Interop.should_not.meta_object?(Object.new)
+  end
+end

src/main/java/org/truffleruby/core/klass/RubyClass.java

@@ -59,11 +59,10 @@ public RubyClass(
         assert !isSingleton || givenBaseName == null;
         this.isSingleton = isSingleton;
         this.attached = attached;
+        this.nonSingletonClass = computeNonSingletonClass(isSingleton, superclass);
+        this.directNonSingletonSubclasses = new ConcurrentWeakSet<>();
 
         if (superclass instanceof RubyClass) {
-            if (lexicalParent == null && givenBaseName != null) {
-                fields.setFullName(givenBaseName);
-            }
             // superclass should be set after "full name"
             this.superclass = superclass;
             this.ancestorClasses = computeAncestorClasses((RubyClass) superclass);
@@ -75,13 +74,13 @@ public RubyClass(
             }
         } else { // BasicObject (nil superclass)
             assert superclass == nil;
+            assert givenBaseName == "BasicObject";
             this.superclass = superclass;
             this.ancestorClasses = EMPTY_CLASS_ARRAY;
             this.depth = 0;
         }
 
-        this.directNonSingletonSubclasses = new ConcurrentWeakSet<>();
-        this.nonSingletonClass = computeNonSingletonClass(isSingleton, superclass);
+        fields.afterConstructed();
     }
 
 
@@ -91,6 +90,7 @@ public RubyClass(
         this.isSingleton = false;
         this.attached = null;
         this.nonSingletonClass = this;
+        this.directNonSingletonSubclasses = new ConcurrentWeakSet<>();
 
         RubyClass basicObjectClass = ClassNodes.createBootClass(language, this, nil, "BasicObject");
         RubyClass objectClass = ClassNodes.createBootClass(language, this, basicObjectClass, "Object");
@@ -100,8 +100,9 @@ public RubyClass(
         this.superclass = superclass;
         this.ancestorClasses = computeAncestorClasses(superclass);
         this.depth = superclass.depth + 1;
-        this.directNonSingletonSubclasses = new ConcurrentWeakSet<>();
         fields.setSuperClass(superclass);
+
+        fields.afterConstructed();
     }
 
     private RubyClass computeNonSingletonClass(boolean isSingleton, Object superclassObject) {

src/main/java/org/truffleruby/core/module/ModuleFields.java

@@ -145,13 +145,27 @@ public ModuleFields(
         classVariables = new ClassVariableStorage(language);
         start = new PrependMarker(this);
         this.includedBy = rubyModule instanceof RubyClass ? null : new ConcurrentWeakSet<>();
+
+        if (lexicalParent == null && givenBaseName != null) {
+            setFullName(givenBaseName);
+        }
+    }
+
+    /** Compute the name eagerly now as it can cause a Shape transition. We need to do so because
+     * InteropLibrary$Asserts.assertMetaObject calls getMetaQualifiedName() and if getName() at that point adds the
+     * object_id property then accepts() asserts that follow will fail. We cannot call this in the ModuleFields
+     * constructor as the name depends on fields of RubyClass (e.g., isSingleton). */
+    public void afterConstructed() {
+        getName();
     }
 
     public RubyConstant getAdoptedByLexicalParent(
             RubyContext context,
             RubyModule lexicalParent,
             String name,
             Node currentNode) {
+        assert name != null;
+
         RubyConstant previous = lexicalParent.fields.setConstantInternal(
                 context,
                 currentNode,
@@ -981,12 +995,10 @@ public ConcurrentMap<RubyModule, RubyModule> getRefinements() {
         return refinements;
     }
 
+    /** Must be called inside the RubyClass constructor */
     public void setSuperClass(RubyClass superclass) {
         assert rubyModule instanceof RubyClass;
         this.parentModule = superclass.fields.start;
-        newMethodsVersion(new ArrayList<>(methods.keySet()));
-        newConstantsVersion(new ArrayList<>(constants.keySet()));
-        newHierarchyVersion();
     }
 
     @Override

src/main/java/org/truffleruby/core/module/ModuleNodes.java

@@ -161,10 +161,11 @@ public static RubyModule createModule(RubyContext context, SourceSection sourceS
                 sourceSection,
                 lexicalParent,
                 name);
+
+        module.fields.afterConstructed();
+
         if (lexicalParent != null) {
             module.fields.getAdoptedByLexicalParent(context, lexicalParent, name, currentNode);
-        } else if (name != null) { // bootstrap module
-            module.fields.setFullName(name);
         }
         return module;
     }

src/main/java/org/truffleruby/core/module/RubyModule.java

@@ -61,10 +61,7 @@ public RubyModule(
 
     protected RubyModule(RubyLanguage language, Shape classShape, String constructorOnlyForClassClass) {
         super(classShape, constructorOnlyForClassClass);
-
-        final ModuleFields fields = new ModuleFields(language, null, null, "Class", this);
-        fields.setFullName("Class");
-        this.fields = fields;
+        this.fields = new ModuleFields(language, null, null, "Class", this);
     }
 
     public String getName() {