Ensure expression and condition coverage is properly mapped to XML output

Signed-off-by: Matthew Ballance <matt.ballance@gmail.com>

Author: mballance

src/ucis/cmd/cmd_convert.py

@@ -7,7 +7,8 @@
 
 def convert(args):
     if args.input_format is None:
-        args.input_format = "xml"
+        rgy_tmp = FormatRgy.inst()
+        args.input_format = rgy_tmp.detectDatabaseFormat(args.input) or "xml"
     if args.output_format is None:
         args.output_format = "xml"
 

src/ucis/mem/mem_fsm_scope.py

@@ -79,16 +79,30 @@ def incrementCount(self, amt: int = 1):
 class MemFSMScope(MemScope):
     """In-memory FSM coverage scope.
 
-    Stores FSM states and transitions in memory. States and transitions
-    are also reflected as FSMBIN cover items for compatibility with
-    generic coverage iteration.
+    Per UCIS LRM section 6.5.6: a UCIS_FSM scope shall have exactly one
+    UCIS_FSM_STATES child scope and one UCIS_FSM_TRANS child scope.
+    FSMBIN coveritems live in those sub-scopes, not directly on the FSM scope.
     """
 
     def __init__(self, parent, name, srcinfo, weight, source, flags=0):
         super().__init__(parent, name, srcinfo, weight, source,
                          ScopeTypeT.FSM, flags)
         self._states = {}         # name -> MemFSMState
         self._transitions = {}    # (from_name, to_name) -> MemFSMTransition
+        # Mandatory sub-scopes (LRM 6.5.6, Figure 36)
+        self._states_scope = MemScope(self, "UCIS:STATE", srcinfo, weight,
+                                      source, ScopeTypeT.FSM_STATES, flags)
+        self.m_children.append(self._states_scope)
+        self._trans_scope = MemScope(self, "UCIS:TRANSITION", srcinfo, weight,
+                                     source, ScopeTypeT.FSM_TRANS, flags)
+        self.m_children.append(self._trans_scope)
+
+    def createNextCover(self, name, data, sourceinfo):
+        """Route FSMBIN coveritems to the correct mandatory sub-scope."""
+        if "->" in name:
+            return self._trans_scope.createNextCover(name, data, sourceinfo)
+        else:
+            return self._states_scope.createNextCover(name, data, sourceinfo)
 
     # --- State API ---
 
@@ -178,8 +192,7 @@ def createNextTransition(self, from_state_name: str, to_state_name: str,
 
     def getIntProperty(self, coverindex, property):
         if property == IntProperty.FSM_STATEVAL:
-            # Return index of state at coverindex if it is a state bin
-            items = list(self.m_cover_items)
+            items = list(self._states_scope.m_cover_items)
             if 0 <= coverindex < len(items):
                 name = items[coverindex].m_name
                 state = self._states.get(name)

src/ucis/merge/db_merger.py

@@ -368,8 +368,29 @@ def _merge_scopes_by_type(self, dst_parent, src_scopes, scope_type):
                 0  # flags - use default
             )
 
-            # Merge coverage items
-            self._merge_code_coverage_items(dst_sub_scope, src_scope_l)
+            if scope_type == ScopeTypeT.FSM:
+                # Per LRM: FSMBIN items live in FSM_STATES/FSM_TRANS sub-scopes.
+                # Collect from src sub-scopes; dst.createNextCover() routes correctly.
+                item_name_m: Dict[str, List] = {}
+                item_name_l = []
+                for src_fsm in src_scope_l:
+                    for sub_type in (ScopeTypeT.FSM_STATES, ScopeTypeT.FSM_TRANS):
+                        for sub_scope in src_fsm.scopes(sub_type):
+                            for ci in sub_scope.coverItems(CoverTypeT.FSMBIN):
+                                nm = ci.getName()
+                                cvg = ci.getCoverData()
+                                if nm not in item_name_m:
+                                    item_name_m[nm] = [0, cvg.goal]
+                                    item_name_l.append(nm)
+                                item_name_m[nm][0] += cvg.data
+                for nm in item_name_l:
+                    count, goal = item_name_m[nm]
+                    cd = CoverData(CoverTypeT.FSMBIN, goal)
+                    cd.data = count
+                    dst_sub_scope.createNextCover(nm, cd, None)
+            else:
+                # Merge coverage items
+                self._merge_code_coverage_items(dst_sub_scope, src_scope_l)
 
     def _merge_code_coverage_items(self, dst_scope, src_scopes):
         """Merge code coverage items from multiple source scopes.

src/ucis/sqlite/sqlite_fsm_scope.py

@@ -26,11 +26,14 @@
 class FSMState:
     """Represents an FSM state"""
 
-    def __init__(self, fsm_scope, state_id: int, state_name: str, state_index: int):
+    def __init__(self, fsm_scope, state_id: int, state_name: str, state_index: int,
+                 cover_scope_id: int = None):
         self.fsm_scope = fsm_scope
         self.state_id = state_id
         self.state_name = state_name
         self.state_index = state_index
+        # scope_id where the FSMBIN coveritem lives (FSM_STATES sub-scope per LRM)
+        self._cover_scope_id = cover_scope_id if cover_scope_id is not None else fsm_scope.scope_id
 
     def getName(self) -> str:
         """Get state name"""
@@ -42,11 +45,10 @@ def getIndex(self) -> int:
 
     def getVisitCount(self) -> int:
         """Get number of times this state was visited"""
-        # Find coveritem for this state
         cursor = self.fsm_scope.ucis_db.conn.execute(
             """SELECT cover_data FROM coveritems 
                WHERE scope_id = ? AND cover_name = ? AND cover_type & 0x800 != 0""",
-            (self.fsm_scope.scope_id, self.state_name)
+            (self._cover_scope_id, self.state_name)
         )
         row = cursor.fetchone()
         return row[0] if row else 0
@@ -61,7 +63,7 @@ def incrementCount(self, amt: int = 1):
         self.fsm_scope.ucis_db.conn.execute(
             """UPDATE coveritems SET cover_data = ?
                WHERE scope_id = ? AND cover_name = ? AND cover_type & 0x800 != 0""",
-            (current + amt, self.fsm_scope.scope_id, self.state_name)
+            (current + amt, self._cover_scope_id, self.state_name)
         )
 
     def incrementVisitCount(self, amt: int = 1):
@@ -109,12 +111,59 @@ def incrementCount(self, amt: int = 1):
 
 
 class SqliteFSMScope(SqliteScope):
-    """FSM coverage scope with state and transition tracking"""
+    """FSM coverage scope with state and transition tracking.
+
+    Per UCIS LRM section 6.5.6: a UCIS_FSM scope shall have exactly one
+    UCIS_FSM_STATES child scope and one UCIS_FSM_TRANS child scope.
+    FSMBIN coveritems are routed to those sub-scopes, not the FSM scope itself.
+    """
 
     def __init__(self, ucis_db, scope_id: int):
         super().__init__(ucis_db, scope_id)
         self._states_cache = {}
         self._transitions_cache = {}
+        self._states_scope_cache = None
+        self._trans_scope_cache = None
+
+    def _get_or_create_sub_scope(self, scope_type, scope_name):
+        """Find existing or create the mandatory FSM sub-scope in the DB."""
+        cursor = self.ucis_db.conn.execute(
+            "SELECT scope_id FROM scopes WHERE parent_id = ? AND scope_type = ?",
+            (self.scope_id, scope_type)
+        )
+        row = cursor.fetchone()
+        if row:
+            return SqliteScope(self.ucis_db, row[0])
+        cursor = self.ucis_db.conn.execute(
+            """INSERT INTO scopes (parent_id, scope_type, scope_name, scope_flags,
+                                   weight, goal, source_file_id, source_line, source_token)
+               VALUES (?, ?, ?, 0, 1, 100, NULL, -1, -1)""",
+            (self.scope_id, scope_type, scope_name)
+        )
+        return SqliteScope(self.ucis_db, cursor.lastrowid)
+
+    def _get_states_scope(self):
+        """Return (and lazy-create) the FSM_STATES child scope."""
+        from ucis.scope_type_t import ScopeTypeT
+        if self._states_scope_cache is None:
+            self._states_scope_cache = self._get_or_create_sub_scope(
+                ScopeTypeT.FSM_STATES, "UCIS:STATE")
+        return self._states_scope_cache
+
+    def _get_trans_scope(self):
+        """Return (and lazy-create) the FSM_TRANS child scope."""
+        from ucis.scope_type_t import ScopeTypeT
+        if self._trans_scope_cache is None:
+            self._trans_scope_cache = self._get_or_create_sub_scope(
+                ScopeTypeT.FSM_TRANS, "UCIS:TRANSITION")
+        return self._trans_scope_cache
+
+    def createNextCover(self, name, data, sourceinfo):
+        """Route FSMBIN coveritems to the correct mandatory sub-scope."""
+        if "->" in name:
+            return self._get_trans_scope().createNextCover(name, data, sourceinfo)
+        else:
+            return self._get_states_scope().createNextCover(name, data, sourceinfo)
 
     def createState(self, state_name: str, state_index: int = None) -> FSMState:
         """Create a new FSM state"""
@@ -135,13 +184,14 @@ def createState(self, state_name: str, state_index: int = None) -> FSMState:
         )
         state_id = cursor.lastrowid
 
-        # Create a coveritem for state visits
+        # Create a coveritem for state visits (routes to FSM_STATES sub-scope)
         cover_data = CoverData(0x800, 0)  # FSMBIN type
         cover_data.data = 0
         state_cover = self.createNextCover(state_name, cover_data, None)
 
-        # Create state object
-        state = FSMState(self, state_id, state_name, state_index)
+        # Create state object; coveritem lives in FSM_STATES sub-scope
+        state = FSMState(self, state_id, state_name, state_index,
+                         cover_scope_id=self._get_states_scope().scope_id)
         self._states_cache[state_name] = state
 
         return state
@@ -161,7 +211,8 @@ def getState(self, state_name: str) -> FSMState:
         if not row:
             return None
 
-        state = FSMState(self, row[0], state_name, row[1])
+        state = FSMState(self, row[0], state_name, row[1],
+                         cover_scope_id=self._get_states_scope().scope_id)
         self._states_cache[state_name] = state
         return state
 
@@ -177,7 +228,9 @@ def getStates(self):
         for row in cursor:
             state_name = row[1]
             if state_name not in self._states_cache:
-                self._states_cache[state_name] = FSMState(self, row[0], state_name, row[2])
+                self._states_cache[state_name] = FSMState(
+                    self, row[0], state_name, row[2],
+                    cover_scope_id=self._get_states_scope().scope_id)
             yield self._states_cache[state_name]
 
     def getNumStates(self) -> int:
@@ -275,12 +328,13 @@ def getStateCoveragePercent(self) -> float:
         if total == 0:
             return 0.0
 
-        # Count states with visits > 0
+        # Coveritems live in FSM_STATES sub-scope per LRM
+        states_scope_id = self._get_states_scope().scope_id
         cursor = self.ucis_db.conn.execute(
             """SELECT COUNT(*) FROM fsm_states fs
-               JOIN coveritems c ON c.scope_id = fs.scope_id AND c.cover_name = fs.state_name
+               JOIN coveritems c ON c.scope_id = ? AND c.cover_name = fs.state_name
                WHERE fs.scope_id = ? AND c.cover_data > 0""",
-            (self.scope_id,)
+            (states_scope_id, self.scope_id)
         )
         row = cursor.fetchone()
         visited = row[0] if row else 0

src/ucis/xml/xml_writer.py

@@ -70,7 +70,7 @@ def write(self, file, db : UCIS, ctx=None):
         self.setAttr(self.root, "writtenBy", wb if wb else getpass.getuser())
         wt = db.getWrittenTime()
         if wt:
-            self.setAttrDateTime(self.root, "writtenTime", str(wt))
+            self.setAttrDateTime(self.root, "writtenTime", wt)
         else:
             self.setAttrDateTime(self.root, "writtenTime",
                                  date.today().strftime("%Y%m%d%H%M%S"))
@@ -208,15 +208,7 @@ def write_instance_coverages(self, s, parent_instance_id=None):
         self.write_branch_coverage(inst, s)
         self.write_fsm_coverage(inst, s)
         self.write_assertion_coverage(inst, s)
-        # Warn once per instance if condition/expression scopes are present
-        warned = False
-        for _ in s.scopes(ScopeTypeT.COND):
-            if not warned and self.ctx is not None:
-                self.ctx.warn(
-                    "xml: condition/expression coverage is not supported "
-                    "— scopes skipped")
-            warned = True
-            break
+        self.write_condition_coverage(inst, s)
         self.write_user_attrs(inst, s)
 
         # Recursively write child instances
@@ -241,7 +233,10 @@ def write_toggle_coverage(self, inst_elem, scope):
                 tb_elem.set("key", "0")
                 for bin_item in bins:
                     name = bin_item.getName()
-                    if "to" in name.lower():
+                    if "->" in name:
+                        parts = name.split("->", 1)
+                        from_val, to_val = parts[0].strip(), parts[1].strip()
+                    elif "to" in name.lower():
                         parts = name.lower().split("to", 1)
                         from_val, to_val = parts[0], parts[1]
                     else:
@@ -300,27 +295,31 @@ def write_fsm_coverage(self, inst_elem, scope):
             fsm_elem.set("name", fsm_scope.getScopeName())
             fsm_elem.set("type", "reg")
             fsm_elem.set("width", "1")
-            bins = list(fsm_scope.coverItems(CoverTypeT.FSMBIN))
+            # FSMBIN items live in FSM_STATES and FSM_TRANS child scopes (LRM 6.5.6)
+            state_bins = []
+            for ss in fsm_scope.scopes(ScopeTypeT.FSM_STATES):
+                state_bins.extend(ss.coverItems(CoverTypeT.FSMBIN))
+            trans_bins = []
+            for ts in fsm_scope.scopes(ScopeTypeT.FSM_TRANS):
+                trans_bins.extend(ts.coverItems(CoverTypeT.FSMBIN))
             # States must come before transitions in XML (schema order)
-            for bin_item in bins:
-                if "->" not in bin_item.getName():
-                    state_elem = self.mkElem(fsm_elem, "state")
-                    state_elem.set("stateName", bin_item.getName())
-                    state_elem.set("stateValue", str(bins.index(bin_item)))
-                    sb_elem = self.mkElem(state_elem, "stateBin")
-                    contents = self.mkElem(sb_elem, "contents")
-                    contents.set("coverageCount", str(bin_item.getCoverData().data))
-            for bin_item in bins:
-                if "->" in bin_item.getName():
-                    parts = bin_item.getName().split("->", 1)
-                    trans_elem = self.mkElem(fsm_elem, "stateTransition")
-                    from_elem = self.mkElem(trans_elem, "state")
-                    from_elem.text = parts[0]
-                    to_elem = self.mkElem(trans_elem, "state")
-                    to_elem.text = parts[1]
-                    tb_elem = self.mkElem(trans_elem, "transitionBin")
-                    contents = self.mkElem(tb_elem, "contents")
-                    contents.set("coverageCount", str(bin_item.getCoverData().data))
+            for i, bin_item in enumerate(state_bins):
+                state_elem = self.mkElem(fsm_elem, "state")
+                state_elem.set("stateName", bin_item.getName())
+                state_elem.set("stateValue", str(i))
+                sb_elem = self.mkElem(state_elem, "stateBin")
+                contents = self.mkElem(sb_elem, "contents")
+                contents.set("coverageCount", str(bin_item.getCoverData().data))
+            for bin_item in trans_bins:
+                parts = bin_item.getName().split("->", 1)
+                trans_elem = self.mkElem(fsm_elem, "stateTransition")
+                from_elem = self.mkElem(trans_elem, "state")
+                from_elem.text = parts[0].strip() if len(parts) > 1 else bin_item.getName()
+                to_elem = self.mkElem(trans_elem, "state")
+                to_elem.text = parts[1].strip() if len(parts) > 1 else ""
+                tb_elem = self.mkElem(trans_elem, "transitionBin")
+                contents = self.mkElem(tb_elem, "contents")
+                contents.set("coverageCount", str(bin_item.getCoverData().data))
         self.write_user_attrs(fc_elem, scope)
 
     def write_assertion_coverage(self, inst_elem, scope):
@@ -360,6 +359,44 @@ def write_assertion_coverage(self, inst_elem, scope):
                                  str(sum(b.getCoverData().data for b in bins)))
         self.write_user_attrs(ac_elem, scope)
 
+    def write_condition_coverage(self, inst_elem, scope):
+        cond_scopes = list(scope.scopes(ScopeTypeT.COND))
+        if not cond_scopes:
+            return
+        cc_elem = self.mkElem(inst_elem, "conditionCoverage")
+        cc_elem.set("metricMode", "UCIS:STD")
+        for key, cond_scope in enumerate(cond_scopes):
+            src = cond_scope.getSourceInfo()
+            if src and src.file:
+                file_id = self.file_id_m.get(src.file.getFileName(), 1)
+                line = src.line if src.line >= 1 else 1
+            else:
+                file_id = 1
+                line = 1
+            uid = f"#cond#{file_id}#{line}#{key}#"
+            expr_elem = self.mkElem(cc_elem, "expr")
+            expr_elem.set("name", uid)
+            expr_elem.set("key", str(key))
+            expr_elem.set("exprString", cond_scope.getScopeName())
+            expr_elem.set("index", "0")
+            expr_elem.set("width", "1")
+            self.addId(expr_elem, src)
+            bins = list(cond_scope.coverItems(CoverTypeT.CONDBIN))
+            # Infer input count from max length of binary-vector bin names
+            binary_names = [b.getName() for b in bins
+                            if all(c in '01-' for c in b.getName())]
+            n_inputs = max((len(n) for n in binary_names), default=1) if binary_names else 1
+            # Emit placeholder subExpr names (signal names not available in DM)
+            for i in range(n_inputs):
+                sub_elem = self.mkElem(expr_elem, "subExpr")
+                sub_elem.text = f"_input_{i}"
+            for bin_item in bins:
+                bin_elem = self.mkElem(expr_elem, "bin")
+                bin_elem.set("alias", bin_item.getName())
+                contents_elem = self.mkElem(bin_elem, "contents")
+                contents_elem.set("coverageCount", str(bin_item.getCoverData().data))
+        self.write_user_attrs(cc_elem, scope)
+
     def write_covergroups(self, inst, scope):
         for cg in scope.scopes(ScopeTypeT.COVERGROUP):
             cgElem = self.mkElem(inst, "covergroupCoverage")
@@ -425,14 +462,15 @@ def write_coverpoint_bins(self, cpElem, coveritems : Iterator[CoverIndex]):
             cpBinElem = self.mkElem(cpElem, "coverpointBin")
             self.setAttr(cpBinElem, "name", cp_bin.getName())
 
-            if cp_bin.data.type == CoverTypeT.CVGBIN:
+            bin_type = cp_bin.getCoverData().type
+            if bin_type == CoverTypeT.CVGBIN:
                 self.setAttr(cpBinElem, "type", "bins")
-            elif cp_bin.data.type == CoverTypeT.IGNOREBIN:
+            elif bin_type == CoverTypeT.IGNOREBIN:
                 self.setAttr(cpBinElem, "type", "ignore")
-            elif cp_bin.data.type == CoverTypeT.ILLEGALBIN:
+            elif bin_type == CoverTypeT.ILLEGALBIN:
                 self.setAttr(cpBinElem, "type", "illegal")
             else:
-                raise Exception("Unknown bin type %s" % str(cp_bin.type))
+                raise Exception("Unknown bin type %s" % str(bin_type))
             self.setAttr(cpBinElem, "key", "0")
 
             # Now, add the coverage data