Amazingly almost everything still passes (spec:ruby:fast totally does).

This start changing all defined methods to use the raw (8859_1) string as the key in the method table. Symbol StringLiteral both now properly use proper ByteList and does not muck with String.
jruby · Dec 13, 2017 · 58557e8 · 58557e8
1 parent 59e8a6a
commit 58557e8
Showing 12 changed files with 75 additions and 54 deletions.
diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -110,6 +110,7 @@
 import org.jruby.runtime.profile.MethodEnhancer;
 import org.jruby.util.ClassProvider;
 import org.jruby.util.IdUtil;
+import org.jruby.util.StringSupport;
 import org.jruby.util.TypeConverter;
 import org.jruby.util.cli.Options;
 import org.jruby.util.collections.WeakHashSet;
@@ -2839,9 +2840,11 @@ public RubyModule private_class_method(IRubyObject[] args) {
 
     @JRubyMethod(name = "alias_method", required = 2, visibility = PRIVATE)
     public RubyModule alias_method(ThreadContext context, IRubyObject newId, IRubyObject oldId) {
-        String newName = newId.asJavaString();
-        defineAlias(newName, oldId.asJavaString());
         RubySymbol newSym = TypeConverter.checkID(newId);
+        RubySymbol oldSym = TypeConverter.checkID(oldId); //  MRI uses rb_to_id but we return existing symbol
+
+        defineAlias(newSym.getRawString(), oldSym.getRawString());
+
         if (isSingleton()) {
             ((MetaClass)this).getAttached().callMethod(context, "singleton_method_added", newSym);
         } else {

diff --git a/core/src/main/java/org/jruby/RubySymbol.java b/core/src/main/java/org/jruby/RubySymbol.java
@@ -147,6 +147,13 @@ public final String asJavaString() {
         return symbol;
     }
 
+    /**
+     * Return a raw (ISO-8859_1) string for use with our method tables etc.
+     */
+    public String getRawString() {
+        return StringSupport.bytelistAsBinaryString(symbolBytes);
+    }
+
     @Override
     public final String toString() {
         return symbol;
@@ -219,6 +226,10 @@ public static RubySymbol newSymbol(Ruby runtime, ByteList bytes) {
         return runtime.getSymbolTable().getSymbol(bytes, false);
     }
 
+    public static RubySymbol newHardSymbol(Ruby runtime, ByteList bytes) {
+        return runtime.getSymbolTable().getSymbol(bytes, true);
+    }
+
     public static RubySymbol newHardSymbol(Ruby runtime, String name) {
         return runtime.getSymbolTable().getSymbol(name, true);
     }

diff --git a/core/src/main/java/org/jruby/ast/LiteralNode.java b/core/src/main/java/org/jruby/ast/LiteralNode.java
@@ -33,6 +33,10 @@ public String getName() {
         return StringSupport.byteListAsString(name);
     }
 
+    public ByteList getByteName() {
+        return name;
+    }
+
     /**
      * Accept for the visitor pattern.
      * @param iVisitor the visitor

diff --git a/core/src/main/java/org/jruby/ast/MethodDefNode.java b/core/src/main/java/org/jruby/ast/MethodDefNode.java
@@ -88,6 +88,7 @@ public Node getBodyNode() {
      * Gets the name.
      * @return Returns a String
      */
+    @Deprecated
     public String getName() {
         return StringSupport.byteListAsString(name);
     }

diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -2083,16 +2083,14 @@ private IRMethod defineNewMethod(MethodDefNode defNode, boolean isInstanceMethod
     public Operand buildDefn(MethodDefNode node) { // Instance method
         IRMethod method = defineNewMethod(node, true);
         addInstr(new DefineInstanceMethodInstr(method));
-        // FIXME: Method name should save encoding
-        return new Symbol(method.getName(), USASCIIEncoding.INSTANCE);
+        return new Symbol(method.getByteName());
     }
 
     public Operand buildDefs(DefsNode node) { // Class method
         Operand container =  build(node.getReceiverNode());
         IRMethod method = defineNewMethod(node, false);
         addInstr(new DefineClassMethodInstr(container, method));
-        // FIXME: Method name should save encoding
-        return new Symbol(method.getName(), USASCIIEncoding.INSTANCE);
+        return new Symbol(method.getByteName());
     }
 
     protected LocalVariable getArgVariable(ByteList name, int depth) {
@@ -3052,7 +3050,7 @@ public Operand buildIter(final IterNode iterNode) {
     }
 
     public Operand buildLiteral(LiteralNode literalNode) {
-        return new StringLiteral(literalNode.getName());
+        return new StringLiteral(literalNode.getByteName());
     }
 
     public Operand buildLocalAsgn(LocalAsgnNode localAsgnNode) {

diff --git a/core/src/main/java/org/jruby/ir/instructions/CallBase.java b/core/src/main/java/org/jruby/ir/instructions/CallBase.java
@@ -47,7 +47,7 @@ protected CallBase(Operation op, CallType callType, ByteList name, Operand recei
         hasClosure = closure != null;
         this.name = name;
         this.callType = callType;
-        this.callSite = getCallSiteFor(callType, StringSupport.byteListAsString(name), potentiallyRefined);
+        this.callSite = getCallSiteFor(callType, StringSupport.bytelistAsBinaryString(name), potentiallyRefined);
         splatMap = IRRuntimeHelpers.buildSplatMap(args);
         flagsComputed = false;
         canBeEval = true;

diff --git a/core/src/main/java/org/jruby/ir/operands/FrozenString.java b/core/src/main/java/org/jruby/ir/operands/FrozenString.java
@@ -43,6 +43,10 @@ protected FrozenString(String string, ByteList bytelist, int coderange, String f
         this.line = line;
     }
 
+    public FrozenString(ByteList bytelist) {
+        this(internedStringFromByteList(bytelist), bytelist);
+    }
+
     /**
      * IRBuild.buildGetDefinition returns a frozen string and this is for all intern'd Java strings.
      */

diff --git a/core/src/main/java/org/jruby/ir/operands/StringLiteral.java b/core/src/main/java/org/jruby/ir/operands/StringLiteral.java
@@ -42,6 +42,10 @@ public StringLiteral(String s) {
         this.frozenString = new FrozenString(s);
     }
 
+    public StringLiteral(ByteList bytes) {
+        this.frozenString = new FrozenString(bytes);
+    }
+
     private StringLiteral(FrozenString frozenString) {
         this.frozenString = frozenString;
     }

diff --git a/core/src/main/java/org/jruby/ir/operands/Symbol.java b/core/src/main/java/org/jruby/ir/operands/Symbol.java
@@ -1,35 +1,25 @@
 package org.jruby.ir.operands;
 
 import org.jcodings.Encoding;
-import org.jcodings.specific.ASCIIEncoding;
 import org.jruby.RubyString;
 import org.jruby.RubySymbol;
 import org.jruby.ir.IRVisitor;
 import org.jruby.ir.persistence.IRReaderDecoder;
 import org.jruby.ir.persistence.IRWriterEncoder;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.util.ByteList;
-import org.jruby.util.io.EncodingUtils;
 
 public class Symbol extends ImmutableLiteral {
-    public static final Symbol KW_REST_ARG_DUMMY = new Symbol("", ASCIIEncoding.INSTANCE);
+    public static final Symbol KW_REST_ARG_DUMMY = new Symbol(new ByteList());
 
     private final ByteList bytes;
 
-    public Symbol(String name, Encoding encoding) {
-        super();
-
-        this.bytes = new ByteList(name.getBytes(EncodingUtils.charsetForEncoding(encoding)), encoding);
-    }
-
     public Symbol(ByteList bytes) {
         this.bytes = bytes;
     }
 
     public boolean equals(Object other) {
-        if (!(other instanceof Symbol)) return false;
-
-        return bytes.equals(((Symbol) other).bytes);
+        return other instanceof Symbol && bytes.equals(((Symbol) other).bytes);
     }
 
     @Override
@@ -65,12 +55,12 @@ public String toString() {
     @Override
     public void encode(IRWriterEncoder e) {
         super.encode(e);
-        e.encode(getString());
-        e.encode(getEncoding());
+
+        e.encode(getBytes());
     }
 
     public static Symbol decode(IRReaderDecoder d) {
-        return new Symbol(d.decodeString(), d.decodeEncoding());
+        return new Symbol(d.decodeByteList());
     }
 
     @Override

diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
@@ -751,13 +751,12 @@ public static IRubyObject nthMatch(ThreadContext context, int matchNumber) {
         return RubyRegexp.nth_match(matchNumber, context.getBackRef());
     }
 
-    public static void defineAlias(ThreadContext context, IRubyObject self, DynamicScope currDynScope, IRubyObject newNameString, IRubyObject oldNameString) {
+    public static void defineAlias(ThreadContext context, IRubyObject self, DynamicScope currDynScope, IRubyObject newName, IRubyObject oldName) {
         if (self == null || self instanceof RubyFixnum || self instanceof RubySymbol) {
             throw context.runtime.newTypeError("no class to make alias");
         }
 
-        RubyModule module = findInstanceMethodContainer(context, currDynScope, self);
-        module.alias_method(context, newNameString, oldNameString);
+        findInstanceMethodContainer(context, currDynScope, self).alias_method(context, newName, oldName);
     }
 
     public static RubyModule getModuleFromScope(ThreadContext context, StaticScope scope, IRubyObject arg) {
@@ -1371,6 +1370,7 @@ public static RubyModule newRubyClassFromIR(Ruby runtime, IRScope irClassBody, O
 
     @Interp
     public static void defInterpretedClassMethod(ThreadContext context, IRScope method, IRubyObject obj) {
+        RubySymbol methodName = RubySymbol.newHardSymbol(context.runtime, method.getByteName());
         RubyClass rubyClass = checkClassForDef(context, method, obj);
 
         DynamicMethod newMethod;
@@ -1379,22 +1379,21 @@ public static void defInterpretedClassMethod(ThreadContext context, IRScope meth
         } else {
             newMethod = new MixedModeIRMethod(method, Visibility.PUBLIC, rubyClass);
         }
-        // FIXME: needs checkID and proper encoding to force hard symbol
-        rubyClass.addMethod(method.getName(), newMethod);
-        if (!rubyClass.isRefinement()) {
-            obj.callMethod(context, "singleton_method_added", context.runtime.fastNewSymbol(method.getName()));
-        }
+
+        rubyClass.addMethod(methodName.getRawString(), newMethod);
+        if (!rubyClass.isRefinement()) obj.callMethod(context, "singleton_method_added", methodName);
     }
 
     @JIT
     public static void defCompiledClassMethod(ThreadContext context, MethodHandle handle, IRScope method, IRubyObject obj) {
+        RubySymbol methodName = RubySymbol.newHardSymbol(context.runtime, method.getByteName());
         RubyClass rubyClass = checkClassForDef(context, method, obj);
 
         // FIXME: needs checkID and proper encoding to force hard symbol
-        rubyClass.addMethod(method.getName(), new CompiledIRMethod(handle, method, Visibility.PUBLIC, rubyClass, method.receivesKeywordArgs()));
+        rubyClass.addMethod(methodName.getRawString(), new CompiledIRMethod(handle, method, Visibility.PUBLIC, rubyClass, method.receivesKeywordArgs()));
         if (!rubyClass.isRefinement()) {
             // FIXME: needs checkID and proper encoding to force hard symbol
-            obj.callMethod(context, "singleton_method_added", context.runtime.fastNewSymbol(method.getName()));
+            obj.callMethod(context, "singleton_method_added", methodName);
         }
     }
 
@@ -1423,48 +1422,50 @@ private static RubyClass checkClassForDef(ThreadContext context, IRScope method,
     @Interp
     public static void defInterpretedInstanceMethod(ThreadContext context, IRScope method, DynamicScope currDynScope, IRubyObject self) {
         Ruby runtime = context.runtime;
+        RubySymbol methodName = RubySymbol.newHardSymbol(runtime, method.getByteName());
         RubyModule rubyClass = findInstanceMethodContainer(context, currDynScope, self);
 
         Visibility currVisibility = context.getCurrentVisibility();
-        Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, rubyClass, method.getName(), currVisibility);
+        Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, rubyClass, methodName, currVisibility);
 
         DynamicMethod newMethod;
-        if (context.runtime.getInstanceConfig().getCompileMode() == RubyInstanceConfig.CompileMode.OFF) {
+        if (runtime.getInstanceConfig().getCompileMode() == RubyInstanceConfig.CompileMode.OFF) {
             newMethod = new InterpretedIRMethod(method, newVisibility, rubyClass);
         } else {
             newMethod = new MixedModeIRMethod(method, newVisibility, rubyClass);
         }
 
-        // FIXME: needs checkID and proper encoding to force hard symbol
-        Helpers.addInstanceMethod(rubyClass, method.getName(), newMethod, currVisibility, context, runtime);
+        Helpers.addInstanceMethod(rubyClass, methodName, newMethod, currVisibility, context, runtime);
     }
 
     @JIT
     public static void defCompiledInstanceMethod(ThreadContext context, MethodHandle handle, IRScope method, DynamicScope currDynScope, IRubyObject self) {
         Ruby runtime = context.runtime;
+        RubySymbol methodName = RubySymbol.newHardSymbol(runtime, method.getByteName());
         RubyModule clazz = findInstanceMethodContainer(context, currDynScope, self);
 
         Visibility currVisibility = context.getCurrentVisibility();
-        Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, clazz, method.getName(), currVisibility);
+        Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, clazz, methodName, currVisibility);
 
         DynamicMethod newMethod = new CompiledIRMethod(handle, method, newVisibility, clazz, method.receivesKeywordArgs());
 
         // FIXME: needs checkID and proper encoding to force hard symbol
-        Helpers.addInstanceMethod(clazz, method.getName(), newMethod, currVisibility, context, runtime);
+        Helpers.addInstanceMethod(clazz, methodName, newMethod, currVisibility, context, runtime);
     }
 
     @JIT
     public static void defCompiledInstanceMethod(ThreadContext context, MethodHandle variable, MethodHandle specific, int specificArity, IRScope method, DynamicScope currDynScope, IRubyObject self) {
         Ruby runtime = context.runtime;
+        RubySymbol methodName = RubySymbol.newHardSymbol(runtime, method.getByteName());
         RubyModule clazz = findInstanceMethodContainer(context, currDynScope, self);
 
         Visibility currVisibility = context.getCurrentVisibility();
-        Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, clazz, method.getName(), currVisibility);
+        Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, clazz, methodName, currVisibility);
 
         DynamicMethod newMethod = new CompiledIRMethod(variable, specific, specificArity, method, newVisibility, clazz, method.receivesKeywordArgs());
 
         // FIXME: needs checkID and proper encoding to force hard symbol
-        Helpers.addInstanceMethod(clazz, method.getName(), newMethod, currVisibility, context, runtime);
+        Helpers.addInstanceMethod(clazz, methodName, newMethod, currVisibility, context, runtime);
     }
 
     @JIT

diff --git a/core/src/main/java/org/jruby/runtime/Helpers.java b/core/src/main/java/org/jruby/runtime/Helpers.java
@@ -1578,27 +1578,25 @@ private static IRubyObject[] argsCatToArgumentsCommon(IRubyObject[] args, IRubyO
         return args;
     }
 
+    @Deprecated
     public static RubySymbol addInstanceMethod(RubyModule containingClass, String name, DynamicMethod method, Visibility visibility, ThreadContext context, Ruby runtime) {
-        containingClass.addMethod(name, method);
-
-        RubySymbol sym = runtime.fastNewSymbol(name);
+        return addInstanceMethod(containingClass, runtime.fastNewSymbol(name), method, visibility, context, runtime);
+    }
 
-        if (!containingClass.isRefinement()) {
-            callNormalMethodHook(containingClass, context, sym);
-        }
+    public static RubySymbol addInstanceMethod(RubyModule containingClass, RubySymbol symbol, DynamicMethod method, Visibility visibility, ThreadContext context, Ruby runtime) {
+        containingClass.addMethod(symbol.getRawString(), method);
 
-        if (visibility == Visibility.MODULE_FUNCTION) {
-            addModuleMethod(containingClass, name, method, context, sym);
-        }
+        if (!containingClass.isRefinement()) callNormalMethodHook(containingClass, context, symbol);
+        if (visibility == Visibility.MODULE_FUNCTION) addModuleMethod(containingClass, method, context, symbol);
 
-        return sym;
+        return symbol;
     }
 
-    private static void addModuleMethod(RubyModule containingClass, String name, DynamicMethod method, ThreadContext context, RubySymbol sym) {
+    private static void addModuleMethod(RubyModule containingClass, DynamicMethod method, ThreadContext context, RubySymbol sym) {
         DynamicMethod singletonMethod = method.dup();
         singletonMethod.setImplementationClass(containingClass.getSingletonClass());
         singletonMethod.setVisibility(Visibility.PUBLIC);
-        containingClass.getSingletonClass().addMethod(name, singletonMethod);
+        containingClass.getSingletonClass().addMethod(sym.getRawString(), singletonMethod);
         containingClass.callMethod(context, "singleton_method_added", sym);
     }
 
@@ -1670,7 +1668,10 @@ public static StaticScope decodeScopeAndDetermineModule(ThreadContext context, S
         return scope;
     }
 
-    public static Visibility performNormalMethodChecksAndDetermineVisibility(Ruby runtime, RubyModule clazz, String name, Visibility visibility) throws RaiseException {
+    public static Visibility performNormalMethodChecksAndDetermineVisibility(Ruby runtime, RubyModule clazz,
+                                                                             RubySymbol symbol, Visibility visibility) throws RaiseException {
+        String name = symbol.asJavaString(); // We just assume simple ascii string since that is all we are examining.
+
         if (clazz == runtime.getDummy()) {
             throw runtime.newTypeError("no class/module to add method");
         }

diff --git a/core/src/main/java/org/jruby/util/StringSupport.java b/core/src/main/java/org/jruby/util/StringSupport.java
@@ -2540,4 +2540,8 @@ public static String byteListAsString(ByteList bytes) {
 
         return new String(bytes.unsafeBytes(), bytes.begin(), bytes.realSize());
     }
+
+    public static String bytelistAsBinaryString(ByteList bytelist) {
+        return new String(bytelist.unsafeBytes(), bytelist.begin(), bytelist.realSize(), ASCIIEncoding.INSTANCE.getCharset());
+    }
 }