Repository: carbon-language/carbon-lang
File: toolchain/check/type_structure.cpp (C++)

Code before:
    // If one structure ends sooner than the other, they don't match.
    if (lhs_cursor == lhs.end() || rhs_cursor == rhs.end()) {
      return false;
    }
    // Same structural element on both sides, they match and both are consumed.
    if (*lhs_cursor == *rhs_cursor) {
      ++lhs_cursor;
      ++rhs_cursor;
      continue;
    }
    // If the element on each side is concrete but they not the same structural
    // shape, then the structures don't match.
    if (*lhs_cursor != Structural::Symbolic &&
        *rhs_cursor != Structural::Symbolic) {
      return false;
    }

    // From here we know one side is a Symbolic and the other is not. We can
    // match the Symbolic against either a single Concrete or a larger bracketed
    // set of Concrete structural elements.
    //
    // We move the symbolic to the RHS to make only one case to handle below,
    // and make sure the cursors are swapped back at the bottom of the loop
    // iteration, so that they compare with the correct `end()`s.
    auto unswap =
        llvm::make_scope_exit([&] { std::swap(lhs_cursor, rhs_cursor); });
    if (*lhs_cursor == Structural::Symbolic) {
      std::swap(lhs_cursor, rhs_cursor);
    } else {
      unswap.release();
    }

    // Consume the symbolic on the RHS.
    ++rhs_cursor;

    // There's a Concrete on the LHS; it matches with the Symbolic and is
    // consumed.
    if (*lhs_cursor == Structural::Concrete) {
      ++lhs_cursor;
      continue;
    }

    // The symbolic on the RHS is in the same position as a close paren on the
    // LHS, which means the structures can not match.
    //
    // Example:
    // - ((c))
    // - ((c?))
    if (*lhs_cursor == Structural::ConcreteCloseParen) {
      return false;
    }

Review comment (suggestion):
I wonder if this would be simpler if you factor the rest of this out to a function / lambda and pass in either `lhs_cursor, rhs_cursor` or `rhs_cursor, lhs_cursor` by reference.

Code after:
    // If one structure ends sooner than the other, they don't match.
    if (lhs_cursor == lhs.end() || rhs_cursor == rhs.end()) {
      return false;
    }
    // Same structural element on both sides, they match and both are consumed.
    //
    // TODO: If we kept the constant value of the concrete element in the type
    // structure, then we could compare them and use that to eliminate matching
    // impls that are not actually compatible.
    if (*lhs_cursor == *rhs_cursor) {
      ++lhs_cursor;
      ++rhs_cursor;
      continue;
    }
    // If the element on each side is concrete but they not the same structural
    // shape, then the structures don't match.
    if (*lhs_cursor != Structural::Symbolic &&
        *rhs_cursor != Structural::Symbolic) {
      return false;
    }

    // From here we know one side is a Symbolic and the other is not. We can
    // match the Symbolic against either a single Concrete or a larger bracketed
    // set of Concrete structural elements.

    // Returns false if the lhs and rhs can not match, true if we should
    // continue checking for compatibility.
    auto consume_symbolic = [](const auto*& lhs_cursor,
                               const auto*& rhs_cursor) -> bool {
      // Consume the symbolic on the RHS.
      ++rhs_cursor;

      // The symbolic on the RHS is in the same position as a close paren on the
      // LHS, which means the structures can not match.
      //
      // Example:
      // - ((c))
      // - ((c?))
      if (*lhs_cursor == Structural::ConcreteCloseParen) {
        return false;
      }

      // There's either a Concrete element or an open paren on the LHS. If it's
      // the former, the Symbolic just matches with it. If it's the latter, the
      // Symbolic matches with everything on the LHS up to the matching closing
      // paren.
      CARBON_CHECK(*lhs_cursor == Structural::Concrete ||
                   *lhs_cursor == Structural::ConcreteOpenParen);
      int depth = 0;
      do {
        switch (*lhs_cursor) {