Fix incorrectly ordered properties for some nested multi.def. symbols

_propagate_deps() visits menu nodes roughly breadth-first, meaning
properties on symbols and choices defined in multiple locations could
end up in the wrong order when copied from the menu node for some
unlucky if/menu nestings.

Fix it by moving the menu-node-to-symbol/choice property copying in
_finalize_tree() so that it's guaranteed to happen in definition order.

This bug was introduced by commit 63a4418 ("Record which MenuNode has
each property").

3 files changed, 213 insertions, 39 deletions

diff --git a/kconfiglib.py b/kconfiglib.py
index 6abb4e6..af5415d 100644
--- a/kconfiglib.py
+++ b/kconfiglib.py
@@ -2097,7 +2097,7 @@ class Kconfig(object):
     def _parse_properties(self, node):
         # Parses and adds properties to the MenuNode 'node' (type, 'prompt',
         # 'default's, etc.) Properties are later copied up to symbols and
-        # choices in a separate pass after parsing, in _propagate_deps().
+        # choices in a separate pass after parsing, in _copy_deps_to_sc().
         #
         # An older version of this code added properties directly to symbols
         # and choices instead of to their menu nodes (and handled dependency
@@ -2536,17 +2536,28 @@ class Kconfig(object):
             if node.item == MENU:
                 visible_if = self._make_and(visible_if, node.visibility)
 
+            # Propagate the menu node's dependencies to each child menu node.
+            #
+            # The recursive _finalize_tree() calls assume that the current
+            # "level" in the tree has already had dependencies propagated. This
+            # makes e.g. implicit submenu creation, which needs to look ahead,
+            # easier to implement.
             self._propagate_deps(node, visible_if)
 
+            # Finalize the children
             cur = node.list
             while cur:
                 self._finalize_tree(cur, visible_if)
                 cur = cur.next
 
         elif isinstance(node.item, Symbol):
-            # The menu node is a symbol. See if we can create an implicit menu
-            # rooted at it and finalize each child in that menu if so, like for
-            # the choice/menu/if case above.
+            # Add the node's non-node-specific properties (defaults, ranges,
+            # etc.) to the Symbol
+            self._copy_deps_to_sc(node)
+
+            # See if we can create an implicit menu rooted at the Symbol and
+            # finalize each child menu node in that menu if so, like for the
+            # choice/menu/if case above
             cur = node
             while cur.next and _auto_menu_dep(node, cur.next):
                 # This also makes implicit submenu creation work recursively,
@@ -2572,17 +2583,12 @@ class Kconfig(object):
         # Empty choices (node.list None) are possible, so this needs to go
         # outside
         if isinstance(node.item, Choice):
+            # Add the node's non-node-specific properties to the choice
+            self._copy_deps_to_sc(node)
             _finalize_choice(node)
 
     def _propagate_deps(self, node, visible_if):
-        # This function combines two tasks:
-        #
-        #   1) Copy properties from menu nodes to symbols and choices
-        #
-        #   2) Propagate dependencies from 'if' and 'depends on' to all
-        #      properties
-        #
-        # See _parse_properties() as well.
+        # Propagates 'node's dependencies to its child menu nodes
 
         # If the parent node holds a Choice, we use the Choice itself as the
         # parent dependency. This makes sense as the value (mode) of the choice
@@ -2605,15 +2611,6 @@ class Kconfig(object):
             if isinstance(cur.item, (Symbol, Choice)):
                 sc = cur.item
 
-                # See the Symbol class docstring
-                sc.direct_dep = self._make_or(sc.direct_dep, dep)
-
-                # TODO: Profile this code and see if the 'if's are worthwhile.
-                # Another potential optimization would be to assign the lists
-                # from the MenuNode directly instead of using extend() in cases
-                # where a symbol/choice only has a single MenuNode (the
-                # majority of cases).
-
                 # Propagate 'visible if' dependencies to the prompt
                 if cur.prompt:
                     cur.prompt = (cur.prompt[0],
@@ -2624,45 +2621,69 @@ class Kconfig(object):
                 if cur.defaults:
                     cur.defaults = [(default, self._make_and(cond, dep))
                                     for default, cond in cur.defaults]
-                    sc.defaults.extend(cur.defaults)
 
                 # Propagate dependencies to ranges
 
                 if cur.ranges:
                     cur.ranges = [(low, high, self._make_and(cond, dep))
                                   for low, high, cond in cur.ranges]
-                    sc.ranges.extend(cur.ranges)
 
                 # Propagate dependencies to selects
 
                 if cur.selects:
                     cur.selects = [(target, self._make_and(cond, dep))
                                    for target, cond in cur.selects]
-                    sc.selects.extend(cur.selects)
-
-                    # Modify the reverse dependencies of the selected symbol
-                    for target, cond in cur.selects:
-                        target.rev_dep = self._make_or(
-                            target.rev_dep,
-                            self._make_and(sc, cond))
 
                 # Propagate dependencies to implies
 
                 if cur.implies:
                     cur.implies = [(target, self._make_and(cond, dep))
                                    for target, cond in cur.implies]
-                    sc.implies.extend(cur.implies)
-
-                    # Modify the weak reverse dependencies of the implied
-                    # symbol
-                    for target, cond in cur.implies:
-                        target.weak_rev_dep = self._make_or(
-                            target.weak_rev_dep,
-                            self._make_and(sc, cond))
 
 
             cur = cur.next
 
+    def _copy_deps_to_sc(self, node):
+        # Copies properties from the menu node 'node' up to its
+        # contained symbol or choice.
+        #
+        # This can't be rolled into _propagate_deps(), because that function
+        # traverses the menu tree roughly breadth-first order, meaning
+        # properties on symbols and choices defined in multiple locations could
+        # end up in the wrong order.
+
+        # Symbol or choice
+        sc = node.item
+
+        # See the Symbol class docstring
+        sc.direct_dep = self._make_or(sc.direct_dep, node.dep)
+
+        if node.defaults:
+            sc.defaults.extend(node.defaults)
+
+        if node.ranges:
+            sc.ranges.extend(node.ranges)
+
+        if node.selects:
+            sc.selects.extend(node.selects)
+
+            # Modify the reverse dependencies of the selected symbol
+            for target, cond in node.selects:
+                target.rev_dep = self._make_or(
+                    target.rev_dep,
+                    self._make_and(sc, cond))
+
+        if node.implies:
+            sc.implies.extend(node.implies)
+
+            # Modify the weak reverse dependencies of the implied
+            # symbol
+            for target, cond in node.implies:
+                target.weak_rev_dep = self._make_or(
+                    target.weak_rev_dep,
+                    self._make_and(sc, cond))
+
+
     #
     # Misc.
     #
diff --git a/tests/Kdepcopy b/tests/Kdepcopy
new file mode 100644
index 0000000..689bcc3
--- /dev/null
+++ b/tests/Kdepcopy
@@ -0,0 +1,125 @@
+# We verify that the properties below end up in definition order
+
+config MULTIDEF
+    bool
+    default A
+    default B
+    select AA
+    imply AA
+
+if FOO
+
+config MULTIDEF
+    default C
+    default D
+    select BB
+    imply BB
+
+if BAR
+
+config MULTIDEF
+    default E
+    default F
+    select CC
+    imply CC
+
+menu "menu"
+
+config MULTIDEF
+    default G
+    default H
+    select DD
+    imply DD
+
+config MULTIDEF
+    default I
+    default J
+    select EE
+    imply EE
+
+endmenu
+
+config MULTIDEF
+    default K
+    default L
+    select FF
+    imply FF
+
+config MULTIDEF
+    default M
+    default N
+    select GG
+    imply GG
+
+endif
+
+config MULTIDEF
+    default O
+    default P
+    select HH
+    select II
+    imply HH
+    imply II
+
+endif
+
+config MULTIDEF
+    default Q
+    default R
+    select JJ
+    imply JJ
+
+
+# Same test with choices involved
+
+config MULTIDEF_CHOICE
+    bool
+    select A
+
+choice
+    bool "choice"
+
+config MULTIDEF_CHOICE
+    bool "multidef choice"
+    select B
+
+endchoice
+
+config MULTIDEF_CHOICE
+    bool
+    select C
+
+
+# Same test with ranges involved
+
+config MULTIDEF_RANGE
+    int
+    range A _
+
+menu "menu"
+
+config MULTIDEF_RANGE
+    int
+    range B _
+
+if FOO
+
+config MULTIDEF_RANGE
+    int
+    range C _
+
+endif
+
+config MULTIDEF_RANGE
+    int
+    range D _
+
+endmenu
+
+config MULTIDEF_RANGE
+    int
+    range E _
+
+config MULTIDEF_RANGE
+    int
+    range F _
diff --git a/testsuite.py b/testsuite.py
index 5c12bc0..c773b99 100644
--- a/testsuite.py
+++ b/testsuite.py
@@ -1022,7 +1022,7 @@ g
     verify_deps(c.syms["JUST_DEPENDS_ON_REFS"].nodes[0], "A", "B")
 
     verify_deps(c.syms["LOTS_OF_REFS"].nodes[0],
-                *(chr(n) for n in range(ord('A'), ord('Z') + 1)))
+                *(chr(n) for n in range(ord("A"), ord("Z") + 1)))
 
     verify_deps(c.syms["INT_REFS"].nodes[0],
                 "A", "B", "C", "D", "E", "F", "G", "H", "y")
@@ -1945,6 +1945,34 @@ g
     verify_is_normal_choice_symbol("WS9")
 
 
+    print("Testing multi.def. property copying")
+
+    c = Kconfig("Kconfiglib/tests/Kdepcopy", warn=False)
+
+    def verify_props(desc, props, prop_names):
+        actual = [prop[0].name for prop in props]
+        expected = prop_names.split()
+
+        verify(actual == expected,
+               "Wrong {} properties, expected '{}', got '{}'"
+               .format(desc, expected, actual))
+
+    verify_props("default", c.syms["MULTIDEF"].defaults,
+                 "A B C D E F G H I J K L M N O P Q R")
+
+    verify_props("select", c.syms["MULTIDEF"].selects,
+                 "AA BB CC DD EE FF GG HH II JJ")
+
+    verify_props("imply", c.syms["MULTIDEF"].selects,
+                 "AA BB CC DD EE FF GG HH II JJ")
+
+    verify_props("select", c.syms["MULTIDEF_CHOICE"].selects,
+                 "A B C")
+
+    verify_props("range", c.syms["MULTIDEF_RANGE"].ranges,
+                 "A B C D E F")
+
+
     print("\nAll selftests passed\n" if all_passed else
           "\nSome selftests failed\n")