Merge branch 'mr/pmderodat/2-bind-arg-order' into 'master'

pmderodat · pmderodat · commit 77dd3ca0182b · 2024-09-13T08:24:16.000Z
Refactor the order of arguments for Bind (Python DSL) and %eq (Lkt)

See merge request eng/libadalang/langkit!1208
diff --git a/langkit/dsl_unparse.py b/langkit/dsl_unparse.py
@@ -1524,7 +1524,7 @@ def handle_designated_env_macro():
 
     elif isinstance(expr, Bind):
         return prio, "%eq({})".format(", ".join(keep([
-            ee_prio(P.lowest, expr.from_expr), ee_prio(P.lowest, expr.to_expr),
+            ee_prio(P.lowest, expr.to_expr), ee_prio(P.lowest, expr.from_expr),
             "conv_prop={}".format(fqn(expr.conv_prop)) if expr.conv_prop else "",
             "logic_ctx={}".format(ee_prio(P.lowest, expr.logic_ctx)) if expr.logic_ctx else "",
         ])))
diff --git a/langkit/expressions/logic.py b/langkit/expressions/logic.py
@@ -198,7 +198,6 @@ def construct_propagate(
             # the support library and the other is generated), they have the
             # same structure, so we cast the given one to the expected type.
             assert len(arg_vars) == 1
-            constructor_name = "Solver.Create_N_Propagate"
             var_array_expr = SavedExpr("Logic_Vars", arg_vars[0])
             saved_exprs.append(var_array_expr)
             var_array = LiteralExpr(
@@ -208,34 +207,44 @@ def construct_propagate(
             var_length = LiteralExpr(
                 "{}.N", None, [var_array_expr.result_var_expr]
             )
+            constructor_name = "Solver.Create_N_Propagate"
             constructor_args = [
+                # "To" argument
                 dest_var,
+                # "Comb" argument
                 cls.functor_expr(
                     f"Logic_Functor_{prop.uid}", prop, var_length
                 )
                 if prop else
                 "Solver_Ifc.No_Combiner",
+                # "Logic_Vars" argument
                 var_array,
             ]
         elif len(arg_vars) == 1:
             constructor_name = "Solver.Create_Propagate"
             constructor_args = [
-                dest_var,
+                # "From" argument
                 arg_vars[0],
+                # "To" argument
+                dest_var,
+                # "Conv" argument
                 cls.functor_expr(f"Logic_Functor_{prop.uid}", prop)
                 if prop else
                 "Solver_Ifc.No_Converter",
             ]
         else:
             constructor_name = "Solver.Create_N_Propagate"
             constructor_args = [
+                # "To" argument
                 dest_var,
+                # "Comb" argument
                 cls.functor_expr(
                     f"Logic_Functor_{prop.uid}", prop,
                     IntegerLiteralExpr(len(arg_vars))
                 )
                 if prop else
                 "Solver_Ifc.No_Combiner",
+                # "Logic_Vars" argument
                 aggregate_expr(
                     type=None,
                     assocs=[(str(i), v) for i, v in enumerate(arg_vars, 1)],
@@ -314,12 +323,12 @@ class Bind(AbstractExpression):
         Bind(A, B, conv_prop=T.TypeOfA.some_property)
     """
 
-    def __init__(self, from_expr, to_expr, conv_prop=None, logic_ctx=None):
+    def __init__(self, to_expr, from_expr, conv_prop=None, logic_ctx=None):
         """
-        :param AbstractExpression from_expr: An expression resolving to a
-            logical variable that is the source of the bind.
         :param AbstractExpression to_expr: An expression resolving to a
             logical variable that is the destination of the bind.
+        :param AbstractExpression from_expr: An expression resolving to a
+            logical variable that is the source of the bind.
         :param PropertyDef|None conv_prop: The property to apply on the
             value of from_expr that will yield the value to give to to_expr.
             For convenience, it can be a property on any subclass of the root
@@ -329,8 +338,8 @@ def __init__(self, from_expr, to_expr, conv_prop=None, logic_ctx=None):
             LogicContext.
         """
         super().__init__()
-        self.from_expr = from_expr
         self.to_expr = to_expr
+        self.from_expr = from_expr
         self.conv_prop = conv_prop
         self.logic_ctx = logic_ctx
 
@@ -442,20 +451,20 @@ def _construct_logic_var(var_expr) -> ResolvedExpression:
         """
         return ResetLogicVar(construct(var_expr, T.LogicVar))
 
-    def construct(self):
+    def construct(self) -> ResolvedExpression:
         # Resolve the converter property, make sure it has an acceptable
         # signature and generate a functor for it.
         self.conv_prop = self._resolve_property(
             "Bind's conv_prop", self.conv_prop, 1
         )
 
-        # Left operand must be a logic variable. Make sure the resulting
+        # The "to" operand must be a logic variable. Make sure the resulting
         # equation will work on a clean logic variable.
-        lhs = self._construct_logic_var(self.from_expr)
+        to_expr = self._construct_logic_var(self.to_expr)
 
-        # Second one can be either a logic variable or an entity (or an AST
+        # The "from" one can be either a logic variable or an entity (or a bare
         # node that is promoted to an entity).
-        rhs = construct(self.to_expr)
+        from_expr = construct(self.from_expr)
 
         logic_ctx = None
         if self.logic_ctx:
@@ -465,45 +474,54 @@ def construct(self):
                 f"Expected LogicContext, got {logic_ctx.type.dsl_name}"
             )
 
-        if rhs.type.matches(T.LogicVar):
+        if from_expr.type.matches(T.LogicVar):
             # The second operand is a logic variable: this is a Propagate or a
             # Unify equation depending on whether we have a conversion
             # property.
 
             # For this operand too, make sure it will work on a clean logic
             # variable.
-            rhs = ResetLogicVar(rhs)
+            from_expr = ResetLogicVar(from_expr)
 
             return (
                 PropagateExpr.construct_propagate(
-                    lhs, [rhs], self.conv_prop, logic_ctx, abstract_expr=self
+                    to_expr,
+                    [from_expr],
+                    self.conv_prop,
+                    logic_ctx,
+                    abstract_expr=self,
                 )
                 if self.conv_prop else
-                UnifyExpr(lhs, rhs, logic_ctx, abstract_expr=self)
+                UnifyExpr(to_expr, from_expr, logic_ctx, abstract_expr=self)
             )
 
         else:
             # The second operand is a value: this is an Assign equation
 
-            if rhs.type.matches(T.root_node):
+            if from_expr.type.matches(T.root_node):
                 from langkit.expressions import make_as_entity
-                rhs = make_as_entity(rhs)
+                from_expr = make_as_entity(from_expr)
             else:
                 check_source_language(
-                    rhs.type.matches(T.root_node.entity)
-                    or rhs.type.matches(T.LogicVar),
+                    from_expr.type.matches(T.root_node.entity)
+                    or from_expr.type.matches(T.LogicVar),
                     "Right operand must be either a logic variable or an"
-                    f" entity, got {rhs.type.dsl_name}"
+                    f" entity, got {from_expr.type.dsl_name}"
                 )
 
             # Because of Ada OOP typing rules, for code generation to work
-            # properly, make sure the type of `rhs` is the root node entity.
-            if rhs.type is not T.root_node.entity:
+            # properly, make sure the type of `from_expr` is the root node
+            # entity.
+            if from_expr.type is not T.root_node.entity:
                 from langkit.expressions import Cast
-                rhs = Cast.Expr(rhs, T.root_node.entity)
+                from_expr = Cast.Expr(from_expr, T.root_node.entity)
 
             return AssignExpr(
-                lhs, rhs, self.conv_prop, logic_ctx, abstract_expr=self
+                to_expr,
+                from_expr,
+                self.conv_prop,
+                logic_ctx,
+                abstract_expr=self,
             )
 
 
diff --git a/langkit/lkt_lowering.py b/langkit/lkt_lowering.py
@@ -1512,8 +1512,8 @@ def match(
 """
 
 eq_signature = FunctionSignature(
-    FunctionParamSpec("from"),
     FunctionParamSpec("to"),
+    FunctionParamSpec("from"),
     FunctionParamSpec("conv_prop", optional=True, keyword_only=True),
     FunctionParamSpec("logic_ctx", optional=True, keyword_only=True),
 )
@@ -4570,8 +4570,8 @@ def lower_new(type_ref: TypeRepo.Defer) -> AbstractExpression:
                         args, _ = eq_signature.match(self.ctx, expr)
                         logic_ctx_expr = args.get("logic_ctx")
                         return E.Bind(
-                            lower(args["from"]),
                             lower(args["to"]),
+                            lower(args["from"]),
                             conv_prop=self.resolve_property(
                                 args.get("conv_prop")
                             ),
diff --git a/testsuite/tests/properties/conv_prop_mistype/expected_concrete_syntax.lkt b/testsuite/tests/properties/conv_prop_mistype/expected_concrete_syntax.lkt
@@ -13,19 +13,63 @@ class FooNode implements Node[FooNode] {
 }
 
 class Examples: FooNode {
-    @parse_field e1: RegularExample
-    @parse_field e2: PlusExample
+    @parse_field re: RegularExample
+    @parse_field pe: PlusExample
+
+    |" Helper to assign PlusExample/RegularExample nodes to the corresponding
+    |" logic vars.
+    fun domains(): Equation =
+        %domain(node.re.lvar, [node.re]) and %domain(node.pe.lvar, [node.pe])
+
+    @exported
+    fun fail_bind_conv_prop(): Bool = (
+        %eq(node.pe.lvar, node.re.lvar, conv_prop=PlusExample.conv_prop) and node.domains()
+    ).solve()
+
+    @exported
+    fun fail_nprop_single(): Bool = (
+        %propagate(node.pe.lvar, PlusExample.conv_prop, node.re.lvar) and node.domains()
+    ).solve()
+
+    @exported
+    fun ok_nprop_multi(): Bool = (
+        %propagate(node.pe.lvar, PlusExample.comb_prop, node.pe.lvar, node.re.lvar, node.pe.lvar) and node.domains()
+    ).solve()
+
+    @exported
+    fun fail_nprop_multi_1(): Bool = (
+        %propagate(node.pe.lvar, PlusExample.comb_prop, node.re.lvar, node.re.lvar, node.pe.lvar) and node.domains()
+    ).solve()
+
+    @exported
+    fun fail_nprop_multi_2(): Bool = (
+        %propagate(node.pe.lvar, PlusExample.comb_prop, node.pe.lvar, node.pe.lvar, node.pe.lvar) and node.domains()
+    ).solve()
+
+    @exported
+    fun fail_nprop_multi_3(): Bool = (
+        %propagate(node.pe.lvar, PlusExample.comb_prop, node.pe.lvar, node.re.lvar, node.re.lvar) and node.domains()
+    ).solve()
+
+    @exported
+    fun ok_nprop_varargs(): Bool = (
+        %propagate(node.pe.lvar, PlusExample.ncomb_prop, [node.pe.lvar, node.pe.lvar]) and node.domains()
+    ).solve()
 
     @exported
-    fun do_solving(): Bool = (
-        (
-            %eq(node.e1.lvar, node.e2.lvar, conv_prop=PlusExample.conv_prop) and %domain(node.e1.lvar, [node.e1])
-        ) and %domain(node.e2.lvar, [node.e2])
+    fun fail_nprop_varargs(): Bool = (
+        %propagate(node.pe.lvar, PlusExample.ncomb_prop, [node.pe.lvar, node.re.lvar]) and node.domains()
     ).solve()
 }
 
 class PlusExample: FooNode {
-    fun conv_prop(): Entity[RegularExample] = null[Entity[RegularExample]]
+    fun conv_prop(): Entity[PlusExample] = self
+
+    fun comb_prop(@ignored re: Entity[RegularExample], pe: Entity[PlusExample]): Entity[PlusExample] =
+        pe
+
+    fun ncomb_prop(pe_list: Array[Entity[PlusExample]]): Entity[PlusExample] =
+        pe_list?[0]
 }
 
 class RegularExample: FooNode {
diff --git a/testsuite/tests/properties/conv_prop_mistype/main.adb b/testsuite/tests/properties/conv_prop_mistype/main.adb
@@ -0,0 +1,55 @@
+with Ada.Exceptions; use Ada.Exceptions;
+with Ada.Text_IO;    use Ada.Text_IO;
+
+with Libfoolang.Analysis; use Libfoolang.Analysis;
+with Libfoolang.Common;   use Libfoolang.Common;
+
+procedure Main is
+   Ctx : constant Analysis_Context := Create_Context;
+   U   : constant Analysis_Unit :=
+     Ctx.Get_From_Buffer ("main.txt", Buffer => "example +example");
+   N   : Examples;
+
+   procedure Check
+     (Label    : String;
+      Property : access function (N : Examples'Class) return Boolean);
+
+   -----------
+   -- Check --
+   -----------
+
+   procedure Check
+     (Label    : String;
+      Property : access function (N : Examples'Class) return Boolean)
+   is
+      Result : Boolean;
+   begin
+      Put_Line ("Calling Examples." & Label & "...");
+      begin
+         Result := Property.all (N);
+         Put_Line ("Result: " & Result'Image);
+      exception
+         when Exc : Property_Error =>
+            Put_Line ("Got a Property_Error: " & Exception_Message (Exc));
+      end;
+      New_Line;
+   end Check;
+
+begin
+   Put_Line ("main.adb: Running...");
+   if U.Has_Diagnostics then
+      raise Program_Error;
+   end if;
+   N := U.Root.As_Examples;
+
+   Check ("p_fail_bind_conv_prop", P_Fail_Bind_Conv_Prop'Access);
+   Check ("p_fail_nprop_single", P_Fail_Nprop_Single'Access);
+   Check ("p_ok_nprop_multi", P_Ok_Nprop_Multi'Access);
+   Check ("p_fail_nprop_multi_1", P_Fail_Nprop_Multi_1'Access);
+   Check ("p_fail_nprop_multi_2", P_Fail_Nprop_Multi_2'Access);
+   Check ("p_fail_nprop_multi_3", P_Fail_Nprop_Multi_3'Access);
+   Check ("p_ok_nprop_varargs", P_Ok_Nprop_Varargs'Access);
+   Check ("p_fail_nprop_varargs", P_Fail_Nprop_Varargs'Access);
+
+   Put_Line ("main.adb: Done.");
+end Main;
diff --git a/testsuite/tests/properties/conv_prop_mistype/main.py b/testsuite/tests/properties/conv_prop_mistype/main.py
diff --git a/testsuite/tests/properties/conv_prop_mistype/test.out b/testsuite/tests/properties/conv_prop_mistype/test.out
@@ -1,5 +1,27 @@
-main.py: Running...
-Evaluating .p_do_solving...
-Got a PropertyError: foo:1:1-1:8: mismatching node type for conv_prop
-main.py: Done.
+main.adb: Running...
+Calling Examples.p_fail_bind_conv_prop...
+Got a Property_Error: main.txt:1:1-1:8: mismatching node type for conv_prop
+
+Calling Examples.p_fail_nprop_single...
+Got a Property_Error: main.txt:1:1-1:8: mismatching node type for conv_prop
+
+Calling Examples.p_ok_nprop_multi...
+Result: TRUE
+
+Calling Examples.p_fail_nprop_multi_1...
+Got a Property_Error: main.txt:1:1-1:8: mismatching node type for comb_prop
+
+Calling Examples.p_fail_nprop_multi_2...
+Got a Property_Error: main.txt:1:9-1:17: mismatching node type for comb_prop
+
+Calling Examples.p_fail_nprop_multi_3...
+Got a Property_Error: main.txt:1:9-1:17: mismatching node type for comb_prop
+
+Calling Examples.p_ok_nprop_varargs...
+Result: TRUE
+
+Calling Examples.p_fail_nprop_varargs...
+Got a Property_Error: main.txt:1:1-1:8: mismatching node type for comb_prop
+
+main.adb: Done.
 Done
diff --git a/testsuite/tests/properties/conv_prop_mistype/test.py b/testsuite/tests/properties/conv_prop_mistype/test.py
