Add comments in YARPTranslator and related classes

Author: andrykonchin

src/main/java/org/truffleruby/language/objects/DefineClassNode.java

@@ -22,6 +22,7 @@
 import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.profiles.ConditionProfile;
 
+/** Defines a new class or returns an existing one found by name in a parent lexical scope. */
 public final class DefineClassNode extends RubyContextSourceNode {
 
     private final String name;

src/main/java/org/truffleruby/language/objects/DefineModuleNode.java

@@ -23,6 +23,7 @@
 import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.profiles.ConditionProfile;
 
+/** Defines a new module or returns an existing one found by name in a parent lexical scope. */
 @NodeChild(value = "lexicalParentModuleNode", type = RubyNode.class)
 public abstract class DefineModuleNode extends RubyContextSourceNode {
 

src/main/java/org/truffleruby/language/objects/RunModuleDefinitionNode.java

@@ -21,6 +21,7 @@
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.IndirectCallNode;
 
+/** Defines a new class or module (or gets an existing one) and executes its body. */
 public final class RunModuleDefinitionNode extends RubyContextSourceNode {
 
     @Child private RubyNode definingModule;

src/main/java/org/truffleruby/language/objects/SingletonClassNode.java

@@ -27,6 +27,7 @@
 import com.oracle.truffle.api.dsl.NodeChild;
 import com.oracle.truffle.api.dsl.Specialization;
 
+/** Get a Ruby object's singleton class */
 // Specializations are order by their frequency on railsbench using --engine.SpecializationStatistics
 @GenerateUncached
 public abstract class SingletonClassNode extends RubyBaseNode {

src/main/java/org/truffleruby/parser/YARPLoadArgumentsTranslator.java

@@ -30,15 +30,17 @@
 import org.truffleruby.language.methods.Arity;
 import org.prism.Nodes;
 
-/** Translates method/block parameters and assign local variables.
+/** Translate method or block parameters and assign local variables.
  *
  * Parameters should be iterated in the same order {@link org.truffleruby.parser.YARPReloadArgumentsTranslator} iterates
  * to handle multiple "_" parameters (and parameters with "_" prefix) correctly. */
 public final class YARPLoadArgumentsTranslator extends YARPBaseTranslator {
 
     private final Arity arity;
-    private final boolean isProc; // block or lambda/method
-    private final boolean isMethod; // method or proc
+    /** block or lambda/method */
+    private final boolean isProc;
+    /** method or proc */
+    private final boolean isMethod;
     private final YARPTranslator yarpTranslator;
 
     private enum State {
@@ -52,6 +54,7 @@ private enum State {
     private int index = 0;
     /** to distinguish pre and post Nodes.RequiredParameterNode parameters */
     private State state;
+    /** number of duplicated parameters with "_" prefix */
     private int repeatedParameterCounter = 2;
 
     public YARPLoadArgumentsTranslator(
@@ -153,7 +156,6 @@ public RubyNode visitMultiTargetNode(Nodes.MultiTargetNode node) {
                             index,
                             hasKeywordArguments(),
                             isProc ? MissingArgumentBehavior.NIL : MissingArgumentBehavior.RUNTIME_ERROR));
-
         } else if (state == YARPLoadArgumentsTranslator.State.POST) {
             readNode = new ReadPostArgumentNode(-index, getRequiredCount(), getOptionalCount(), hasRest(),
                     hasKeywordArguments());
@@ -196,6 +198,7 @@ public RubyNode visitRequiredParameterNode(Nodes.RequiredParameterNode node) {
 
         final int slot;
         if (node.isRepeatedParameter()) {
+            // when there are multiple "_" parameters
             String name = createNameForRepeatedParameter(node.name);
             slot = environment.declareVar(name);
         } else {
@@ -229,6 +232,7 @@ public RubyNode visitOptionalParameterNode(Nodes.OptionalParameterNode node) {
 
         final int slot;
         if (node.isRepeatedParameter()) {
+            // when there are multiple "_" parameters
             String name = createNameForRepeatedParameter(node.name);
             slot = environment.declareVar(name);
         } else {
@@ -249,6 +253,7 @@ public RubyNode visitRestParameterNode(Nodes.RestParameterNode node) {
         final int slot;
         if (node.name != null) {
             if (node.isRepeatedParameter()) {
+                // when there are multiple "_" parameters
                 String name = createNameForRepeatedParameter(node.name);
                 slot = environment.declareVar(name);
             } else {
@@ -339,6 +344,17 @@ private boolean hasRest() {
         return parameters.rest != null;
     }
 
+    /** Generate a name for subsequent local variable, that look like %_1, %_2, etc.
+     *
+     * Parameters that start with "_" (e.g. _, _options, etc.) can be used repeatedly in a method parameters list, e.g.
+     *
+     * <pre>
+     *     def foo(_, _)
+     *     end
+     * </pre>
+     *
+     * They should be forwarded properly but there are no local variables declared for such duplicated parameters.
+     * That's why such local variables should be declared now. */
     private String createNameForRepeatedParameter(String name) {
         int count = repeatedParameterCounter++;
         return Layouts.TEMP_PREFIX + name + count;

src/main/java/org/truffleruby/parser/YARPMultiTargetNodeTranslator.java

@@ -23,15 +23,24 @@
 import org.truffleruby.language.locals.ReadLocalNode;
 import org.truffleruby.language.locals.WriteLocalNode;
 
-// Could be used in ordinal multi-assignment and for destructuring array argument in method/proc parameters:
-// - a, (b, c) = 1, [2, 3]
-// - def foo(a, (b, c)) end
-// NOTE: cannot inherit from YARPBaseTranslator because it returns AssignableNode instead of RubyNode.
+/** Translate Nodes.MultiTargetNode.
+ *
+ * Used in the following cases: descturturing in multi-assignment and destructuring in method or block parameters:
+ *
+ * <pre>
+ *     a, (b, c) = 1, [2, 3]
+ *
+ *     def foo(a, (b, c))
+ *     end
+ * </pre>
+ *
+ * NOTE: cannot inherit from YARPBaseTranslator because it returns AssignableNode instead of RubyNode. */
 public final class YARPMultiTargetNodeTranslator extends AbstractNodeVisitor<AssignableNode> {
 
     private final Nodes.MultiTargetNode node;
     private final RubyLanguage language;
     private final YARPTranslator yarpTranslator;
+    /** a node that will be destructured */
     private final RubyNode readNode;
 
     public YARPMultiTargetNodeTranslator(
@@ -159,12 +168,12 @@ public AssignableNode visitSplatNode(Nodes.SplatNode node) {
         if (node.expression != null) {
             return node.expression.accept(this);
         } else {
+            // do nothing for single "*"
             return new NoopAssignableNode();
         }
     }
 
     @Override
-    // RequiredParameterNode is handled during destructuring method/proc arguments
     public AssignableNode visitRequiredParameterNode(Nodes.RequiredParameterNode node) {
         final String name = node.name;
         final ReadLocalNode lhs = yarpTranslator.getEnvironment().findLocalVarNode(name);

src/main/java/org/truffleruby/parser/YARPMultiWriteNodeTranslator.java

@@ -20,7 +20,9 @@
 import org.truffleruby.core.cast.SplatCastNodeGen;
 import org.truffleruby.language.RubyNode;
 
-// NOTE: cannot inherit from YARPBaseTranslator because it returns AssignableNode instead of RubyNode.
+/** Translate Nodes.MultiWriteNode node.
+ *
+ * NOTE: cannot inherit from YARPBaseTranslator because it returns AssignableNode instead of RubyNode. */
 public final class YARPMultiWriteNodeTranslator extends AbstractNodeVisitor<AssignableNode> {
 
     private final Nodes.MultiWriteNode node;

src/main/java/org/truffleruby/parser/YARPReloadArgumentsTranslator.java

@@ -25,8 +25,8 @@
 
 
 /** Produces code to reload arguments from local variables back into the arguments array. Only works for simple cases.
- * Used for zsuper calls which pass the same arguments, but will pick up modifications made to them in the method so
- * far.
+ * Used for zsuper (a super call without explicit arguments) calls which pass the same arguments, but will pick up
+ * modifications made to them in the method so far.
  *
  * Parameters should be iterated in the same order {@link org.truffleruby.parser.YARPLoadArgumentsTranslator} iterates
  * to handle multiple "_" parameters (and parameters with "_" prefix) correctly. */
@@ -122,8 +122,7 @@ public RubyNode[] reload(Nodes.ParametersNode parameters) {
             } else if (parameters.keyword_rest instanceof Nodes.NoKeywordsParameterNode) {
                 // do nothing
             } else if (parameters.keyword_rest instanceof Nodes.ForwardingParameterNode) {
-                // do nothing - it's already handled in the #reload method
-                // NOTE: don't handle '&' for now as far as anonymous & isn't supported yet
+                // do nothing - it will be handled separately
             } else {
                 throw CompilerDirectives.shouldNotReachHere();
             }
@@ -134,14 +133,16 @@ public RubyNode[] reload(Nodes.ParametersNode parameters) {
         }
 
         if (parameters.keyword_rest instanceof Nodes.ForwardingParameterNode) {
-            // ... parameter (so-called "forward arguments") means there is implicit * parameter
+            // ... parameter means there is implicit * parameter
             restParameterIndex = parameters.requireds.length + parameters.optionals.length;
             var readRestNode = environment.findLocalVarNode(TranslatorEnvironment.FORWARDED_REST_NAME);
             sequence.add(readRestNode);
 
-            // ... parameter (so-called "forward arguments") means there is implicit ** parameter
+            // ... parameter means there is implicit ** parameter
             var readKeyRestNode = environment.findLocalVarNode(TranslatorEnvironment.FORWARDED_KEYWORD_REST_NAME);
             sequence.add(readKeyRestNode);
+
+            // implicit block parameter is handled separately in YARPTranslator#visitForwardingSuperNode
         }
 
         return sequence.toArray(RubyNode.EMPTY_ARRAY);
@@ -152,6 +153,7 @@ public RubyNode visitRequiredParameterNode(Nodes.RequiredParameterNode node) {
         final String name;
 
         if (node.isRepeatedParameter()) {
+            // when there are multiple "_" parameters
             name = createNameForRepeatedParameter(node.name);
         } else {
             name = node.name;
@@ -165,6 +167,7 @@ public RubyNode visitOptionalParameterNode(Nodes.OptionalParameterNode node) {
         final String name;
 
         if (node.isRepeatedParameter()) {
+            // when there are multiple "_" parameters
             name = createNameForRepeatedParameter(node.name);
         } else {
             name = node.name;
@@ -185,6 +188,7 @@ public RubyNode visitRestParameterNode(Nodes.RestParameterNode node) {
 
         if (node.name != null) {
             if (node.isRepeatedParameter()) {
+                // when there are multiple "_" parameters
                 name = createNameForRepeatedParameter(node.name);
             } else {
                 name = node.name;
@@ -228,6 +232,17 @@ protected RubyNode defaultVisit(Nodes.Node node) {
         return yarpTranslator.defaultVisit(node);
     }
 
+    /** Generate a name for subsequent local variable, that look like %_1, %_2, etc.
+     *
+     * Parameters that start with "_" (e.g. _, _options, etc.) can be used repeatedly in a method parameters list, e.g.
+     *
+     * <pre>
+     *     def foo(_, _)
+     *     end
+     * </pre>
+     *
+     * They should be forwarded properly but there are no local variables declared for such duplicated parameters.
+     * That's why such local variables should be declared now. */
     private String createNameForRepeatedParameter(String name) {
         int count = repeatedParameterCounter++;
         return Layouts.TEMP_PREFIX + name + count;