Merge pull request #4022 from ruby/revert-4016-byte_ary_identifiers

Revert "Switch identifiers to byte[]"

Author: kddnewton

templates/java/org/ruby_lang/prism/Loader.java.erb

@@ -1,4 +1,4 @@
-<%- id_type = Prism::Template::JAVA_IDENTIFIER_TYPE -%>
+<%- string_type = Prism::Template::JAVA_STRING_TYPE -%>
 package org.ruby_lang.prism;
 
 import java.lang.Short;
@@ -19,29 +19,37 @@ public class Loader {
 
     // Overridable methods
 
-    <%- if id_type == "String" -%>
-    public abstract <%= id_type %> bytesToName(byte[] bytes);
-    <%- end -%>
+    public Charset getEncodingCharset(String encodingName) {
+        encodingName = encodingName.toLowerCase(Locale.ROOT);
+        if (encodingName.equals("ascii-8bit")) {
+            return StandardCharsets.US_ASCII;
+        }
+        return Charset.forName(encodingName);
+    }
+
+    public <%= string_type %> bytesToName(byte[] bytes) {
+        <%- if string_type == "String" -%>
+        return new String(bytes, encodingCharset).intern();
+        <%- else -%>
+        return null; // Must be implemented by subclassing Loader
+        <%- end -%>
+    }
 
     private static final class ConstantPool {
 
         private final Loader loader;
         private final int bufferOffset;
-        private final <%= id_type %>[] cache;
+        private final <%= string_type %>[] cache;
 
         ConstantPool(Loader loader, int bufferOffset, int length) {
             this.loader = loader;
             this.bufferOffset = bufferOffset;
-            <%- if id_type == "String" -%>
-            cache = new <%= id_type %>[length];
-            <%- else -%>
-            cache = new byte[length][];
-            <%- end -%>
+            cache = new <%= string_type %>[length];
         }
 
-        <%= id_type %> get(ByteBuffer buffer, int oneBasedIndex) {
+        <%= string_type %> get(ByteBuffer buffer, int oneBasedIndex) {
             int index = oneBasedIndex - 1;
-            <%= id_type %> constant = cache[index];
+            <%= string_type %> constant = cache[index];
 
             if (constant == null) {
                 int offset = bufferOffset + index * 8;
@@ -51,11 +59,7 @@ public class Loader {
                 byte[] bytes = new byte[length];
                 buffer.get(start, bytes);
 
-                <%- if id_type == "byte[]" -%>
-                constant = bytes;
-                <%- else %>
                 constant = loader.bytesToName(bytes);
-                <%- end %>
                 cache[index] = constant;
             }
 
@@ -66,6 +70,9 @@ public class Loader {
 
     private final ByteBuffer buffer;
     protected String encodingName;
+    <%- if string_type == "String" -%>
+    private Charset encodingCharset;
+    <%- end -%>
     private ConstantPool constantPool;
     private Nodes.Source source = null;
 
@@ -93,6 +100,9 @@ public class Loader {
         byte[] encodingNameBytes = new byte[encodingLength];
         buffer.get(encodingNameBytes);
         this.encodingName = new String(encodingNameBytes, StandardCharsets.US_ASCII);
+        <%- if string_type == "String" -%>
+        this.encodingCharset = getEncodingCharset(this.encodingName);
+        <%- end -%>
 
         source.setStartLine(loadVarSInt());
         source.setLineOffsets(loadLineOffsets());
@@ -203,11 +213,11 @@ public class Loader {
         }
     }
 
-    private <%= id_type %> loadConstant() {
+    private <%= string_type %> loadConstant() {
         return constantPool.get(buffer, loadVarUInt());
     }
 
-    private <%= id_type %> loadOptionalConstant() {
+    private <%= string_type %> loadOptionalConstant() {
         if (buffer.get(buffer.position()) != 0) {
             return loadConstant();
         } else {
@@ -216,16 +226,12 @@ public class Loader {
         }
     }
 
-    private <%= id_type %>[] loadConstants() {
+    private <%= string_type %>[] loadConstants() {
         int length = loadVarUInt();
         if (length == 0) {
-            return Nodes.EMPTY_IDENTIFIER_ARRAY;
+            return Nodes.EMPTY_STRING_ARRAY;
         }
-        <%- if id_type == "String" -%>
-        <%= id_type %>[] constants = new <%= id_type %>[length];
-        <%- else -%>
-        <%= id_type %>[] constants = new byte[length][];
-        <%- end -%>
+        <%= string_type %>[] constants = new <%= string_type %>[length];
         for (int i = 0; i < length; i++) {
             constants[i] = constantPool.get(buffer, loadVarUInt());
         }
@@ -389,7 +395,7 @@ public class Loader {
         int bufferPosition = buffer.position();
         int serializedLength = buffer.getInt();
         // Load everything except the body and locals, because the name, receiver, parameters are still needed for lazily defining the method
-        Nodes.DefNode lazyDefNode = new Nodes.DefNode(<%= base_params.join(", ") -%>, -bufferPosition, this, loadConstant(), loadOptionalNode(), (Nodes.ParametersNode) loadOptionalNode(), null, Nodes.EMPTY_IDENTIFIER_ARRAY);
+        Nodes.DefNode lazyDefNode = new Nodes.DefNode(<%= base_params.join(", ") -%>, -bufferPosition, this, loadConstant(), loadOptionalNode(), (Nodes.ParametersNode) loadOptionalNode(), null, Nodes.EMPTY_STRING_ARRAY);
         buffer.position(bufferPosition + serializedLength); // skip past the serialized DefNode
         return lazyDefNode;
     }

templates/java/org/ruby_lang/prism/Nodes.java.erb

@@ -1,4 +1,4 @@
-<%- id_type = Prism::Template::JAVA_IDENTIFIER_TYPE -%>
+<%- string_type = Prism::Template::JAVA_STRING_TYPE -%>
 package org.ruby_lang.prism;
 
 import java.lang.Override;
@@ -16,7 +16,7 @@ import java.util.Arrays;
 // @formatter:off
 public abstract class Nodes {
 
-    public static final <%= id_type %>[] EMPTY_IDENTIFIER_ARRAY = {};
+    public static final <%= string_type %>[] EMPTY_STRING_ARRAY = {};
 
     @Target(ElementType.FIELD)
     @Retention(RetentionPolicy.SOURCE)
@@ -383,7 +383,7 @@ public abstract class Nodes {
             builder.append('\n');
             <%- when Prism::Template::ConstantListField -%>
             builder.append('\n');
-            for (<%= id_type %> constant : this.<%= field.name %>) {
+            for (<%= string_type %> constant : this.<%= field.name %>) {
                 builder.append(nextNextIndent).append('"').append(constant).append('"').append('\n');
             }
             <%- when Prism::Template::Flags -%>

templates/template.rb

@@ -11,8 +11,8 @@ module Template # :nodoc: all
     REMOVE_ON_ERROR_TYPES = SERIALIZE_ONLY_SEMANTICS_FIELDS
     CHECK_FIELD_KIND = ENV.fetch("CHECK_FIELD_KIND", false)
 
-    JAVA_BACKEND = ENV["PRISM_JAVA_BACKEND"] || "default"
-    JAVA_IDENTIFIER_TYPE = JAVA_BACKEND == "truffleruby" ? "String" : "byte[]"
+    JAVA_BACKEND = ENV["PRISM_JAVA_BACKEND"] || "truffleruby"
+    JAVA_STRING_TYPE = JAVA_BACKEND == "jruby" ? "org.jruby.RubySymbol" : "String"
     INCLUDE_NODE_ID = !SERIALIZE_ONLY_SEMANTICS_FIELDS || JAVA_BACKEND == "jruby"
 
     COMMON_FLAGS_COUNT = 2
@@ -272,7 +272,7 @@ def call_seq_type
       end
 
       def java_type
-        JAVA_IDENTIFIER_TYPE
+        JAVA_STRING_TYPE
       end
     end
 
@@ -292,7 +292,7 @@ def call_seq_type
       end
 
       def java_type
-        JAVA_IDENTIFIER_TYPE
+        JAVA_STRING_TYPE
       end
     end
 
@@ -312,7 +312,7 @@ def call_seq_type
       end
 
       def java_type
-        "#{JAVA_IDENTIFIER_TYPE}[]"
+        "#{JAVA_STRING_TYPE}[]"
       end
     end