Fix escaping of identifiers in OQ3 output (#9660) (#9673)

Since gh-9100 relaxed the naming restrictions on registers, it is now
necessary to manually escape register names during the export process.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit c29886d)

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

qiskit/qasm3/exporter.py

@@ -13,6 +13,7 @@
 """QASM3 Exporter"""
 
 import collections
+import re
 import io
 import itertools
 import numbers
@@ -108,6 +109,17 @@
     }
 )
 
+# This probably isn't precisely the same as the OQ3 spec, but we'd need an extra dependency to fully
+# handle all Unicode character classes, and this should be close enough for users who aren't
+# actively _trying_ to break us (fingers crossed).
+_VALID_IDENTIFIER = re.compile(r"[\w][\w\d]*", flags=re.U)
+
+
+def _escape_invalid_identifier(name: str) -> str:
+    if name in _RESERVED_KEYWORDS or not _VALID_IDENTIFIER.fullmatch(name):
+        name = "_" + re.sub(r"[^\w\d]", "_", name)
+    return name
+
 
 class Exporter:
     """QASM3 exporter main class."""
@@ -359,9 +371,9 @@ def _register_variable(self, variable, name=None) -> ast.Identifier:
                     f"tried to reserve '{name}', but it is already used by '{table[name]}'"
                 )
         else:
-            name = variable.name
-            while name in table or name in _RESERVED_KEYWORDS:
-                name = f"{variable.name}__generated{next(self._counter)}"
+            name = basename = _escape_invalid_identifier(variable.name)
+            while name in table:
+                name = f"{basename}__generated{next(self._counter)}"
         identifier = ast.Identifier(name)
         table[identifier.string] = variable
         table[variable] = identifier

releasenotes/notes/fix-qasm3-name-escape-43a8b0e5ec59a471.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Register and parameter names will now be escaped during the OpenQASM 3 export
+    (:func:`.qasm3.dumps`) if they are not already valid identifiers.  Fixed `#9658
+    <https://github.com/Qiskit/qiskit-terra/issues/9658>`__.

test/python/circuit/test_circuit_qasm3.py

@@ -75,8 +75,13 @@ class TestCircuitQASM3(QiskitTestCase):
     @classmethod
     def setUpClass(cls):
         # These regexes are not perfect by any means, but sufficient for simple tests on controlled
-        # input circuits.
-        cls.register_regex = re.compile(r"^\s*let\s+(?P<name>\w+\b)", re.U | re.M)
+        # input circuits.  They can allow false negatives (in which case, update the regex), but to
+        # be useful for the tests must _never_ have false positive matches.  We use an explicit
+        # space (`\s`) or semicolon rather than the end-of-word `\b` because we want to ensure that
+        # the exporter isn't putting out invalid characters as part of the identifiers.
+        cls.register_regex = re.compile(
+            r"^\s*(let|bit(\[\d+\])?)\s+(?P<name>\w+)[\s;]", re.U | re.M
+        )
         scalar_type_names = {
             "angle",
             "duration",
@@ -88,7 +93,7 @@ def setUpClass(cls):
         cls.scalar_parameter_regex = re.compile(
             r"^\s*((input|output|const)\s+)?"  # Modifier
             rf"({'|'.join(scalar_type_names)})\s*(\[[^\]]+\])?\s+"  # Type name and designator
-            r"(?P<name>\w+\b)",  # Parameter name
+            r"(?P<name>\w+)[\s;]",  # Parameter name
             re.U | re.M,
         )
         super().setUpClass()
@@ -1390,6 +1395,26 @@ def test_custom_gate_used_in_loop_scope(self):
         )
         self.assertEqual(dumps(qc), expected_qasm)
 
+    def test_registers_have_escaped_names(self):
+        """Test that both types of register are emitted with safely escaped names if they begin with
+        invalid names. Regression test of gh-9658."""
+        qc = QuantumCircuit(
+            QuantumRegister(2, name="q_{reg}"), ClassicalRegister(2, name="c_{reg}")
+        )
+        qc.measure([0, 1], [0, 1])
+        out_qasm = dumps(qc)
+        matches = {match_["name"] for match_ in self.register_regex.finditer(out_qasm)}
+        self.assertEqual(len(matches), 2, msg=f"Observed OQ3 output:\n{out_qasm}")
+
+    def test_parameters_have_escaped_names(self):
+        """Test that parameters are emitted with safely escaped names if they begin with invalid
+        names. Regression test of gh-9658."""
+        qc = QuantumCircuit(1)
+        qc.u(Parameter("p_{0}"), 2 * Parameter("p_?0!"), 0, 0)
+        out_qasm = dumps(qc)
+        matches = {match_["name"] for match_ in self.scalar_parameter_regex.finditer(out_qasm)}
+        self.assertEqual(len(matches), 2, msg=f"Observed OQ3 output:\n{out_qasm}")
+
     def test_parameter_expression_after_naming_escape(self):
         """Test that :class:`.Parameter` instances are correctly renamed when they are used with
         :class:`.ParameterExpression` blocks, even if they have names that needed to be escaped."""
@@ -1401,10 +1426,10 @@ def test_parameter_expression_after_naming_escape(self):
             [
                 "OPENQASM 3;",
                 'include "stdgates.inc";',
-                "input float[64] measure__generated0;",
+                "input float[64] _measure;",
                 "qubit[1] _all_qubits;",
                 "let q = _all_qubits[0:0];",
-                "U(2*measure__generated0, 0, 0) q[0];",
+                "U(2*_measure, 0, 0) q[0];",
                 "",
             ]
         )
@@ -1437,15 +1462,15 @@ def test_reserved_keywords_as_names_are_escaped(self, keyword):
             qc = QuantumCircuit(qreg)
             out_qasm = dumps(qc)
             register_name = self.register_regex.search(out_qasm)
-            self.assertTrue(register_name)
+            self.assertTrue(register_name, msg=f"Observed OQ3:\n{out_qasm}")
             self.assertNotEqual(keyword, register_name["name"])
         with self.subTest("parameter"):
             qc = QuantumCircuit(1)
             param = Parameter(keyword)
             qc.u(param, 0, 0, 0)
             out_qasm = dumps(qc)
             parameter_name = self.scalar_parameter_regex.search(out_qasm)
-            self.assertTrue(parameter_name)
+            self.assertTrue(parameter_name, msg=f"Observed OQ3:\n{out_qasm}")
             self.assertNotEqual(keyword, parameter_name["name"])