Skip to content

Add validation logic on user-provided inputs #1076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mhucka wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Add validation logic on user-provided inputs #1076

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
144fd62
Add validation logic on user-provided inputs
mhucka May 22, 2026
e51ad39
Add additional checks
mhucka May 22, 2026
73920bd
Format
mhucka May 22, 2026
ad4dea1
Add another check
mhucka May 22, 2026
bac9c05
Check that qubits are distinct
mhucka May 22, 2026
996a4bf
Merge branch 'master' into fix-parse-vulnerability
mhucka May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
205 changes: 169 additions & 36 deletions tensorflow_quantum/core/src/circuit_parser_qsim.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,15 @@ inline Status ParseProtoControls(const Operation& op,
return ::tensorflow::Status();
}
bool valid;
unsigned int tmp;
unsigned int tmp = 0;
control_qubits->reserve(control_toks.size());
for (auto tok : control_toks) {
// don't bother error checking since this is done earlier
// in program_resolution.
valid = absl::SimpleAtoi(tok, &tmp);
if (!absl::SimpleAtoi(tok, &tmp) || tmp >= num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid control qubit: " + std::string(tok));
}
Comment on lines +116 to +119
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The validation logic for control qubits should also check for duplicate control qubits and ensure that control qubits do not overlap with the target qubits of the operation. A qubit cannot act as both a control and a target in the same operation.

    if (!absl::SimpleAtoi(tok, &tmp) || tmp >= num_qubits) {
      return Status(absl::StatusCode::kInvalidArgument,
                    "Invalid control qubit: " + std::string(tok));
    }
    for (const auto& q : op.qubits()) {
      unsigned int target_id;
      if (absl::SimpleAtoi(q.id(), &target_id) && target_id == tmp) {
        return Status(absl::StatusCode::kInvalidArgument,
                      "Control qubit overlaps with target qubit: " + std::string(tok));
      }
    }
    for (const auto& existing_q : *control_qubits) {
      if (existing_q == num_qubits - tmp - 1) {
        return Status(absl::StatusCode::kInvalidArgument,
                      "Duplicate control qubit: " + std::string(tok));
      }
    }

control_qubits->push_back(num_qubits - tmp - 1);
}
control_values->reserve(control_v_toks.size());
Expand Down Expand Up @@ -156,8 +159,15 @@ inline Status SingleConstantGate(
const std::function<QsimGate(unsigned int, unsigned int)>& create_f,
const unsigned int num_qubits, const unsigned int time,
QsimCircuit* circuit, std::vector<GateMetaData>* metadata) {
unsigned int q0;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q0);
unsigned int q0 = 0;
if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 >= num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
auto gate = create_f(time, num_qubits - q0 - 1);
Status s = OptionalInsertControls(op, num_qubits, &gate);
if (!s.ok()) {
Expand All @@ -181,9 +191,19 @@ inline Status TwoConstantGate(
create_f,
const unsigned int num_qubits, const unsigned int time,
QsimCircuit* circuit, std::vector<GateMetaData>* metadata) {
unsigned int q0, q1;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q0);
(void)absl::SimpleAtoi(op.qubits(1).id(), &q1);
unsigned int q0 = 0, q1 = 0;
if (op.qubits_size() < 2) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 >= num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
if (!absl::SimpleAtoi(op.qubits(1).id(), &q1) || q1 >= num_qubits || q0 == q1) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid or duplicate qubit id: " + op.qubits(1).id());
}
auto gate = create_f(time, num_qubits - q0 - 1, num_qubits - q1 - 1);
Status s = OptionalInsertControls(op, num_qubits, &gate);
if (!s.ok()) {
Expand All @@ -207,10 +227,17 @@ inline Status SingleEigenGate(
create_f,
const unsigned int num_qubits, const unsigned int time,
QsimCircuit* circuit, std::vector<GateMetaData>* metadata) {
unsigned int q0;
unsigned int q0 = 0;
float exp, exp_s, gs;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q0);
if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 >= num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}

absl::optional<std::string> exponent_symbol;
u = ParseProtoArg(op, "exponent", param_map, &exp, &exponent_symbol);
Expand Down Expand Up @@ -255,11 +282,21 @@ inline Status TwoEigenGate(
float, float)>& create_f,
const unsigned int num_qubits, const unsigned int time,
QsimCircuit* circuit, std::vector<GateMetaData>* metadata) {
unsigned int q0, q1;
unsigned int q0 = 0, q1 = 0;
float exp, exp_s, gs;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q0);
(void)absl::SimpleAtoi(op.qubits(1).id(), &q1);
if (op.qubits_size() < 2) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 >= num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
if (!absl::SimpleAtoi(op.qubits(1).id(), &q1) || q1 >= num_qubits || q0 == q1) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid or duplicate qubit id: " + op.qubits(1).id());
}

absl::optional<std::string> exponent_symbol;
u = ParseProtoArg(op, "exponent", param_map, &exp, &exponent_symbol);
Expand Down Expand Up @@ -394,10 +431,18 @@ inline Status PhasedXGate(const Operation& op, const SymbolMap& param_map,
const unsigned int num_qubits,
const unsigned int time, QsimCircuit* circuit,
std::vector<GateMetaData>* metadata) {
int q0;
int q0 = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other gate functions and to avoid signed/unsigned comparison issues and potential overflow when num_qubits is large.

  unsigned int q0 = 0;

float pexp, pexp_s, exp, exp_s, gs;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q0);
if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 < 0 ||
q0 >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
Comment on lines +441 to +445
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q0 as unsigned int, the check for q0 < 0 is redundant, and the comparison with num_qubits is safer and more consistent.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }


absl::optional<std::string> exponent_symbol;
u = ParseProtoArg(op, "exponent", param_map, &exp, &exponent_symbol);
Expand Down Expand Up @@ -453,11 +498,23 @@ inline Status FsimGate(const Operation& op, const SymbolMap& param_map,
const unsigned int num_qubits, const unsigned int time,
QsimCircuit* circuit,
std::vector<GateMetaData>* metadata) {
int q0, q1;
int q0 = 0, q1 = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other gate functions and to avoid signed/unsigned comparison issues.

  unsigned int q0 = 0, q1 = 0;

float theta, theta_s, phi, phi_s;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q0);
(void)absl::SimpleAtoi(op.qubits(1).id(), &q1);
if (op.qubits_size() < 2) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 < 0 ||
q0 >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
if (!absl::SimpleAtoi(op.qubits(1).id(), &q1) || q1 < 0 ||
q1 >= (int)num_qubits || q0 == q1) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid or duplicate qubit id: " + op.qubits(1).id());
}
Comment thread
mhucka marked this conversation as resolved.
Comment on lines +508 to +517
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q0 and q1 as unsigned int, the checks for negative values are redundant, and the comparisons with num_qubits are safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }
  if (!absl::SimpleAtoi(op.qubits(1).id(), &q1) || q1 >= num_qubits || q0 == q1) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid or duplicate qubit id: " + op.qubits(1).id());
  }


absl::optional<std::string> theta_symbol;
u = ParseProtoArg(op, "theta", param_map, &theta, &theta_symbol);
Expand Down Expand Up @@ -509,11 +566,23 @@ inline Status PhasedISwapGate(const Operation& op, const SymbolMap& param_map,
const unsigned int num_qubits,
const unsigned int time, QsimCircuit* circuit,
std::vector<GateMetaData>* metadata) {
int q0, q1;
int q0 = 0, q1 = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other gate functions and to avoid signed/unsigned comparison issues.

  unsigned int q0 = 0, q1 = 0;

float pexp, pexp_s, exp, exp_s;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q0);
(void)absl::SimpleAtoi(op.qubits(1).id(), &q1);
if (op.qubits_size() < 2) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 < 0 ||
q0 >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
if (!absl::SimpleAtoi(op.qubits(1).id(), &q1) || q1 < 0 ||
q1 >= (int)num_qubits || q0 == q1) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid or duplicate qubit id: " + op.qubits(1).id());
}
Comment thread
mhucka marked this conversation as resolved.
Comment on lines +576 to +585
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q0 and q1 as unsigned int, the checks for negative values are redundant, and the comparisons with num_qubits are safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q0) || q0 >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }
  if (!absl::SimpleAtoi(op.qubits(1).id(), &q1) || q1 >= num_qubits || q0 == q1) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid or duplicate qubit id: " + op.qubits(1).id());
  }


absl::optional<std::string> exponent_symbol;
u = ParseProtoArg(op, "exponent", param_map, &exp, &exponent_symbol);
Expand Down Expand Up @@ -599,10 +668,18 @@ inline Status AsymmetricDepolarizingChannel(const Operation& op,
const unsigned int num_qubits,
const unsigned int time,
NoisyQsimCircuit* ncircuit) {
int q;
int q = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other functions and to avoid signed/unsigned comparison issues.

  unsigned int q = 0;

float p_x, p_y, p_z;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q);
if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q < 0 ||
q >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
Comment on lines +678 to +682
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }


u = ParseProtoArg(op, "p_x", {}, &p_x);
u = ParseProtoArg(op, "p_y", {}, &p_y);
Expand All @@ -620,10 +697,18 @@ inline Status DepolarizingChannel(const Operation& op,
const unsigned int num_qubits,
const unsigned int time,
NoisyQsimCircuit* ncircuit) {
int q;
int q = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other functions and to avoid signed/unsigned comparison issues.

  unsigned int q = 0;

float p;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q);
if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q < 0 ||
q >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
Comment on lines +707 to +711
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }


u = ParseProtoArg(op, "p", {}, &p);
if (!u.ok()) {
Expand All @@ -637,10 +722,18 @@ inline Status DepolarizingChannel(const Operation& op,

inline Status GADChannel(const Operation& op, const unsigned int num_qubits,
const unsigned int time, NoisyQsimCircuit* ncircuit) {
int q;
int q = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other functions and to avoid signed/unsigned comparison issues.

  unsigned int q = 0;

float p, gamma;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q);
if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q < 0 ||
q >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
Comment on lines +732 to +736
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }


u = ParseProtoArg(op, "p", {}, &p);
if (!u.ok()) {
Expand All @@ -660,8 +753,16 @@ inline Status GADChannel(const Operation& op, const unsigned int num_qubits,
inline Status ResetChannel(const Operation& op, const unsigned int num_qubits,
const unsigned int time,
NoisyQsimCircuit* ncircuit) {
int q;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q);
int q = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other functions and to avoid signed/unsigned comparison issues.

  unsigned int q = 0;

if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q < 0 ||
q >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
Comment on lines +761 to +765
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }


auto chan = qsim::Cirq::ResetChannel<float>::Create(time, num_qubits - q - 1);
ncircuit->channels.push_back(chan);
Expand All @@ -672,10 +773,18 @@ inline Status AmplitudeDampingChannel(const Operation& op,
const unsigned int num_qubits,
const unsigned int time,
NoisyQsimCircuit* ncircuit) {
int q;
int q = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other functions and to avoid signed/unsigned comparison issues.

  unsigned int q = 0;

float gamma;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q);
if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q < 0 ||
q >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
Comment on lines +783 to +787
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }


u = ParseProtoArg(op, "gamma", {}, &gamma);
if (!u.ok()) {
Expand All @@ -691,10 +800,18 @@ inline Status PhaseDampingChannel(const Operation& op,
const unsigned int num_qubits,
const unsigned int time,
NoisyQsimCircuit* ncircuit) {
int q;
int q = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other functions and to avoid signed/unsigned comparison issues.

  unsigned int q = 0;

float gamma;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q);
if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q < 0 ||
q >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
Comment on lines +810 to +814
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }


u = ParseProtoArg(op, "gamma", {}, &gamma);
if (!u.ok()) {
Expand All @@ -711,10 +828,18 @@ inline Status PhaseFlipChannel(const Operation& op,
const unsigned int num_qubits,
const unsigned int time,
NoisyQsimCircuit* ncircuit) {
int q;
int q = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other functions and to avoid signed/unsigned comparison issues.

  unsigned int q = 0;

float p;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q);
if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q < 0 ||
q >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
Comment on lines +838 to +842
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }


u = ParseProtoArg(op, "p", {}, &p);
if (!u.ok()) {
Expand All @@ -730,10 +855,18 @@ inline Status PhaseFlipChannel(const Operation& op,
inline Status BitFlipChannel(const Operation& op, const unsigned int num_qubits,
const unsigned int time,
NoisyQsimCircuit* ncircuit) {
int q;
int q = 0;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use unsigned int for qubit IDs to be consistent with other functions and to avoid signed/unsigned comparison issues.

  unsigned int q = 0;

float p;
Status u;
(void)absl::SimpleAtoi(op.qubits(0).id(), &q);
if (op.qubits_size() < 1) {
return Status(absl::StatusCode::kInvalidArgument,
"Missing qubit(s) in gate.");
}
if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q < 0 ||
q >= (int)num_qubits) {
return Status(absl::StatusCode::kInvalidArgument,
"Invalid qubit id: " + op.qubits(0).id());
}
Comment on lines +865 to +869
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With q as unsigned int, the check for q < 0 is redundant, and the comparison with num_qubits is safer.

  if (!absl::SimpleAtoi(op.qubits(0).id(), &q) || q >= num_qubits) {
    return Status(absl::StatusCode::kInvalidArgument,
                  "Invalid qubit id: " + op.qubits(0).id());
  }


u = ParseProtoArg(op, "p", {}, &p);
if (!u.ok()) {
Expand Down
Loading