Move semantic field flag into C sources

Rather than templating two versions of sources with and without
non-semantic fields, we can make that determination at build time.
Passing -DPRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS=1 to `make` will
force that variable to be literally true, and the compiler will
eliminate `if` blocks that use it to conditionally serialize non-
semantic data.

This simplifies the templates by removing that variation and allows
building both forms of the library from a single generated set of
sources.

Author: headius

.github/workflows/java-wasm-bindings.yml

@@ -27,7 +27,7 @@ jobs:
           bundler-cache: true
 
       - name: rake templates
-        run: PRISM_EXCLUDE_PRETTYPRINT=1 PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS=1 bundle exec rake templates
+        run: PRISM_EXCLUDE_PRETTYPRINT=1 PRISM_OMIT_NODE_ID=1 bundle exec rake templates
 
       - name: Set up WASI-SDK
         run: |

Makefile

@@ -56,7 +56,7 @@ java/wasm/src/main/wasm/prism.wasm: Makefile $(SOURCES) $(HEADERS)
 	$(Q) $(MAKEDIRS) $(@D)
 	$(Q) $(WASI_SDK_PATH)/bin/clang \
 		$(DEBUG_FLAGS) \
-		-DPRISM_EXCLUDE_PRETTYPRINT -DPRISM_EXPORT_SYMBOLS -D_WASI_EMULATED_MMAN \
+		-DPRISM_EXCLUDE_PRETTYPRINT -DPRISM_EXPORT_SYMBOLS -D_WASI_EMULATED_MMAN -DPRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS=1 \
 		-lwasi-emulated-mman $(CPPFLAGS) $(JAVA_WASM_CFLAGS) \
 		-Wl,--export-all -Wl,--no-entry -mexec-model=reactor -lc++ -lc++abi \
 		-o $@ $(SOURCES)

rakelib/serialization.rake

@@ -1,12 +1,9 @@
 # frozen_string_literal: true
 
 task "test:java_loader" do
-  # Recompile with PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS=1
-  # Due to some JRuby bug this does not get propagated to the compile task, so require the caller to set the env var
-  # ENV["PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS"] = "1"
-  raise "this task requires $SERIALIZE_ONLY_SEMANTICS_FIELDS to be set" unless ENV["PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS"]
-
   Rake::Task["clobber"].invoke
+  # All Java API consumers want semantic-only build
+  ENV["CFLAGS"] = "-DPRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS=1"
   Rake::Task["test:java_loader:internal"].invoke
 end
 

src/prism.c

@@ -22843,7 +22843,7 @@ pm_serialize_header(pm_buffer_t *buffer) {
     pm_buffer_append_byte(buffer, PRISM_VERSION_MAJOR);
     pm_buffer_append_byte(buffer, PRISM_VERSION_MINOR);
     pm_buffer_append_byte(buffer, PRISM_VERSION_PATCH);
-    pm_buffer_append_byte(buffer, PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS ? 1 : 0);
+    pm_buffer_append_byte(buffer, PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS);
 }
 
 /**

templates/include/prism/ast.h.erb

@@ -268,11 +268,13 @@ PRISM_EXPORTED_FUNCTION pm_<%= node.human %>_t * pm_<%= node.human %>_new(pm_are
 <%- end -%>
 
 /**
- * When we're serializing to Java, we want to skip serializing the location
- * fields as they won't be used by JRuby or TruffleRuby. This boolean allows us
- * to specify that through the environment. It will never be true except for in
- * those build systems.
- */
-#define PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS <%= Prism::Template::SERIALIZE_ONLY_SEMANTICS_FIELDS ? 1 : 0 %>
+* When we're serializing to Java, we want to skip serializing the location
+* fields as they won't be used by JRuby or TruffleRuby. This boolean allows us
+* to specify that through the environment. It will never be true except for in
+* those build systems.
+*/
+#ifndef PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS
+#define PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS 0
+#endif
 
 #endif

templates/java/api/src/main/java-templates/org/ruby_lang/prism/Loader.java.erb

@@ -326,12 +326,12 @@ public class Loader {
     }
 
     <%-
-    base_params = [*("nodeId" if Prism::Template::INCLUDE_NODE_ID), "startOffset", "length"]
+    base_params = [*("nodeId" unless Prism::Template::OMIT_NODE_ID), "startOffset", "length"]
     base_params_sig = base_params.map { "int #{_1}" }.join(", ")
     -%>
     private Nodes.Node loadNode() {
         int type = buffer.get() & 0xFF;
-        <%- if Prism::Template::INCLUDE_NODE_ID -%>
+        <%- unless Prism::Template::OMIT_NODE_ID -%>
         int nodeId = loadVarUInt();
         <%- end -%>
         int startOffset = loadVarUInt();
@@ -343,7 +343,7 @@ public class Loader {
             case <%= index + 1 %>:
             <%-
             params = []
-            params << "nodeId" if Prism::Template::INCLUDE_NODE_ID
+            params << "nodeId" unless Prism::Template::OMIT_NODE_ID
             params << "startOffset" << "length"
             params << "buffer.getInt()" << "null" if node.needs_serialized_length?
             params << "loadFlags()" if node.flags

templates/java/api/src/main/java-templates/org/ruby_lang/prism/Nodes.java.erb

@@ -89,14 +89,14 @@ public abstract class Nodes {
 
         public static final Node[] EMPTY_ARRAY = {};
 
-        <%- if Prism::Template::INCLUDE_NODE_ID -%>
+        <%- unless Prism::Template::OMIT_NODE_ID -%>
         public final int nodeId;
         <%- end -%>
         public final int startOffset;
         public final int length;
         private boolean newLineFlag = false;
 
-        <%- if Prism::Template::INCLUDE_NODE_ID -%>
+        <%- unless Prism::Template::OMIT_NODE_ID -%>
         public Node(int nodeId, int startOffset, int length) {
             this.nodeId = nodeId;
         <%- else -%>
@@ -246,7 +246,7 @@ public abstract class Nodes {
 
         <%-
           params = []
-          params << "int nodeId" if Prism::Template::INCLUDE_NODE_ID
+          params << "int nodeId" unless Prism::Template::OMIT_NODE_ID
           params << "int startOffset" << "int length"
           if node.needs_serialized_length?
             params << "int serializedLength"
@@ -256,7 +256,7 @@ public abstract class Nodes {
           params.concat(node.semantic_fields.map { |field| "#{field.java_type} #{field.name}" })
         -%>
         public <%= node.name -%>(<%= params.join(", ") %>) {
-        <%- if Prism::Template::INCLUDE_NODE_ID -%>
+        <%- unless Prism::Template::OMIT_NODE_ID -%>
             super(nodeId, startOffset, length);
         <%- else -%>
             super(startOffset, length);
@@ -281,7 +281,7 @@ public abstract class Nodes {
 
         public <%= node.name -%> getNonLazy() {
             if (isLazy()) {
-                return loader.createDefNodeFromSavedPosition(<%= "nodeId, " if Prism::Template::INCLUDE_NODE_ID %>startOffset, length, -serializedLength);
+                return loader.createDefNodeFromSavedPosition(<%= "nodeId, " unless Prism::Template::OMIT_NODE_ID %>startOffset, length, -serializedLength);
             } else {
                 return this;
             }

templates/src/serialize.c.erb

@@ -73,7 +73,7 @@ static void
 pm_serialize_node(pm_parser_t *parser, pm_node_t *node, pm_buffer_t *buffer) {
     pm_buffer_append_byte(buffer, (uint8_t) PM_NODE_TYPE(node));
 
-    <%- if Prism::Template::INCLUDE_NODE_ID -%>
+    <%- unless Prism::Template::OMIT_NODE_ID -%>
     pm_buffer_append_varuint(buffer, node->node_id);
     <%- end -%>
     pm_serialize_location(&node->location, buffer);
@@ -91,8 +91,12 @@ pm_serialize_node(pm_parser_t *parser, pm_node_t *node, pm_buffer_t *buffer) {
             size_t length_offset = buffer->length;
             pm_buffer_append_string(buffer, "\0\0\0\0", 4); /* consume 4 bytes, updated below */
             <%- end -%>
-            <%- unless Prism::Template::SERIALIZE_ONLY_SEMANTICS_FIELDS && !node.flags -%>
+            <%- if node.flags -%>
             pm_buffer_append_varuint(buffer, (uint32_t) node->flags);
+            <%- else -%>
+            if (!PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS) {
+                pm_buffer_append_varuint(buffer, (uint32_t) node->flags);
+            }
             <%- end -%>
             <%- node.fields.each do |field| -%>
             <%- case field -%>
@@ -121,17 +125,25 @@ pm_serialize_node(pm_parser_t *parser, pm_node_t *node, pm_buffer_t *buffer) {
                 pm_buffer_append_varuint(buffer, pm_sizet_to_u32(((pm_<%= node.human %>_t *)node)-><%= field.name %>.ids[index]));
             }
             <%- when Prism::Template::LocationField -%>
-            <%- if field.should_be_serialized? -%>
+            <%- unless field.semantic_field? -%>
+            if (!PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS) {
+            <%- end -%>
             pm_serialize_location(&((pm_<%= node.human %>_t *)node)-><%= field.name %>, buffer);
+            <%- unless field.semantic_field? -%>
+            }
             <%- end -%>
             <%- when Prism::Template::OptionalLocationField -%>
-            <%- if field.should_be_serialized? -%>
+            <%- unless field.semantic_field? -%>
+              if (!PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS) {
+            <%- end -%>
             if (((pm_<%= node.human %>_t *)node)-><%= field.name %>.length == 0) {
                 pm_buffer_append_byte(buffer, 0);
             } else {
                 pm_buffer_append_byte(buffer, 1);
                 pm_serialize_location(&((pm_<%= node.human %>_t *)node)-><%= field.name %>, buffer);
             }
+            <%- unless field.semantic_field? -%>
+              }
             <%- end -%>
             <%- when Prism::Template::UInt8Field -%>
             pm_buffer_append_byte(buffer, ((pm_<%= node.human %>_t *)node)-><%= field.name %>);
@@ -261,9 +273,9 @@ pm_serialize_metadata(pm_parser_t *parser, pm_buffer_t *buffer) {
     pm_serialize_encoding(parser->encoding, buffer);
     pm_buffer_append_varsint(buffer, parser->start_line);
     pm_serialize_line_offset_list(&parser->line_offsets, buffer);
-<%- unless Prism::Template::SERIALIZE_ONLY_SEMANTICS_FIELDS -%>
-    pm_serialize_comment_list(&parser->comment_list, buffer);
-<%- end -%>
+    if (!PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS) {
+        pm_serialize_comment_list(&parser->comment_list, buffer);
+    }
     pm_serialize_magic_comment_list(&parser->magic_comment_list, buffer);
     pm_serialize_data_loc(parser, buffer);
     pm_serialize_diagnostic_list(&parser->error_list, buffer);

templates/template.rb

@@ -7,12 +7,11 @@
 
 module Prism
   module Template # :nodoc: all
-    SERIALIZE_ONLY_SEMANTICS_FIELDS = ENV.fetch("PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS", false)
     CHECK_FIELD_KIND = ENV.fetch("CHECK_FIELD_KIND", false)
 
     JAVA_BACKEND = ENV["PRISM_JAVA_BACKEND"] || "default"
     JAVA_IDENTIFIER_TYPE = JAVA_BACKEND == "truffleruby" ? "String" : "byte[]"
-    INCLUDE_NODE_ID = !SERIALIZE_ONLY_SEMANTICS_FIELDS || JAVA_BACKEND == "jruby"
+    OMIT_NODE_ID = ENV.fetch("PRISM_OMIT_NODE_ID", false)
 
     COMMON_FLAGS_COUNT = 2
 
@@ -95,10 +94,6 @@ def each_comment_java_line(&block)
       def semantic_field?
         true
       end
-
-      def should_be_serialized?
-        SERIALIZE_ONLY_SEMANTICS_FIELDS ? semantic_field? : true
-      end
     end
 
     # Some node fields can be specialized if they point to a specific kind of