ronantakizawa/codereview-bench huggingface dataset:
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
    }
  }
  return r;
}

// End of xstrtod code
// ---------------------------------------------------------------------------

void uint_state_init(uint_state *self) {
  self->seen_sint = 0;
  self->seen_uint = 0;
  self->seen_null = 0;
}

int uint64_conflict(uint_state *self) {
  return self->seen_uint && (self->seen_sint || self->seen_null);
}

/**
 * @brief Check if the character in the pointer indicates a number.
 * It expects that you consumed all leading whitespace.
 *
 * @param p_item Pointer to verify
 * @return Non-zero integer indicating that has a digit 0 otherwise.
 */
static inline bool has_digit_int(const char *str) {
  if (!str || *str == '\0') {
    return false;
  }

  switch (*str) {
  case '0':
  case '1':
  case '2':
  case '3':
  case '4':
  case '5':
  case '6':
  case '7':
  case '8':
  case '9':
    return true;
  case '+':
  case '-':
    return str[1] != '\0' && isdigit_ascii(str[1]);
  default:
    return false;
  }
}

static inline bool has_only_spaces(const char *str) {

Review comment (suggestion):
Going back to my comment made (in another PR?) - if we replace the `char **` words member with a custom struct, we could add to that struct a member that flags this as the word is built, rather than coming back after the fact.

Not something to tackle in this PR - just wanted to give you an idea

Code after:
    }
  }
  return r;
}

// End of xstrtod code
// ---------------------------------------------------------------------------

void uint_state_init(uint_state *self) {
  self->seen_sint = 0;
  self->seen_uint = 0;
  self->seen_null = 0;
}

int uint64_conflict(uint_state *self) {
  return self->seen_uint && (self->seen_sint || self->seen_null);
}

/* Copy a string without `char_to_remove` into `output`.
 */
static int copy_string_without_char(char output[PROCESSED_WORD_CAPACITY],
                                    const char *str, size_t str_len,
                                    char char_to_remove) {
  const char *left = str;
  const char *end_ptr = str + str_len;
  size_t bytes_written = 0;

  while (left < end_ptr) {
    const size_t remaining_bytes_to_read = end_ptr - left;
    const char *right = memchr(left, char_to_remove, remaining_bytes_to_read);

    if (!right) {
      // If it doesn't find the char to remove, just copy until EOS.
      right = end_ptr;
    }

    const size_t chunk_size = right - left;

    if (chunk_size + bytes_written >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    memcpy(&output[bytes_written], left, chunk_size);
    bytes_written += chunk_size;

    left = right + 1;
  }

  output[bytes_written] = '\0';
  return 0;
}
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
                                    char char_to_remove) {
  const char *left = str;
  const char *right;
  const char *end_ptr = str + str_len;
  size_t bytes_read = 0;

  while ((right = memchr(left, char_to_remove, str_len - bytes_read)) != NULL) {
    size_t nbytes = right - left;

    // check if we have enough space, including the null terminator.
    if (nbytes + bytes_read >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    // copy block
    memcpy(&output[bytes_read], left, nbytes);
    bytes_read += nbytes;
    left = right + 1;

    // Exit after processing the entire string
    if (left >= end_ptr) {
      break;
    }
  }

  // copy final chunk that doesn't contain char_to_remove
  if (end_ptr > left) {
    size_t nbytes = end_ptr - left;
    if (nbytes + bytes_read >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    memcpy(&output[bytes_read], left, nbytes);
    bytes_read += nbytes;
  }

  // null terminate
  output[bytes_read] = '\0';
  return 0;
}

int64_t str_to_int64(const char *p_item, int64_t int_min, int64_t int_max,
                     int *error, char tsep) {
  if (!p_item || *p_item == '\0') {
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  while (isspace_ascii(*p_item)) {
    ++p_item;
  }

  if (!has_digit_int(p_item)) {

Review comment (suggestion):
Another nit but I think you can do `str_len - bytes_read` here to get everything that's left and avoid the `if` statement here altogether (which may helpful for branch prediction)

```c
memcpy(&output[bytes_written], left, str_len - bytes_read)
```
(requires the aforementioned fix to `bytes_read`)

Code after:
  // Handle sign.
  const char *digit_start = has_sign ? p + 1 : p;

  // Check that there is a first digit.
  if (!isdigit_ascii(*digit_start)) {
    // Error...
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  char buffer[PROCESSED_WORD_CAPACITY];
  const size_t str_len = strlen(p);
  if (tsep != '\0' && memchr(p, tsep, str_len) != NULL) {
    const int status = copy_string_without_char(buffer, p, str_len, tsep);
    if (status != 0) {
      // Word is too big, probably will cause an overflow
      *error = ERROR_OVERFLOW;
      return 0;
    }
    p = buffer;
  }

  char *endptr;
  int64_t number = strtoll(p, &endptr, 10);

  if (errno == ERANGE || number > int_max || number < int_min) {
    *error = ERROR_OVERFLOW;
    errno = 0;
    return 0;
  }

  // Skip trailing spaces.
  while (isspace_ascii(*endptr)) {
    ++endptr;
  }

  // Did we use up all the characters?
  if (*endptr) {
    *error = ERROR_INVALID_CHARS;
    return 0;
  }

  *error = 0;
  return number;
}

uint64_t str_to_uint64(uint_state *state, const char *p_item, int64_t int_max,
                       uint64_t uint_max, int *error, char tsep) {
  const char *p = p_item;
  // Skip leading spaces.
  while (isspace_ascii(*p)) {
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
    if (nbytes + bytes_read >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    // copy block
    memcpy(&output[bytes_read], left, nbytes);
    bytes_read += nbytes;
    left = right + 1;

    // Exit after processing the entire string
    if (left >= end_ptr) {
      break;
    }
  }

  // copy final chunk that doesn't contain char_to_remove
  if (end_ptr > left) {
    size_t nbytes = end_ptr - left;
    if (nbytes + bytes_read >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    memcpy(&output[bytes_read], left, nbytes);
    bytes_read += nbytes;
  }

  // null terminate
  output[bytes_read] = '\0';
  return 0;
}

int64_t str_to_int64(const char *p_item, int64_t int_min, int64_t int_max,
                     int *error, char tsep) {
  if (!p_item || *p_item == '\0') {
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  while (isspace_ascii(*p_item)) {
    ++p_item;
  }

  if (!has_digit_int(p_item)) {
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  errno = 0;
  char buffer[PROCESSED_WORD_CAPACITY];
  size_t str_len = strlen(p_item);
  if (tsep != '\0' && memchr(p_item, tsep, str_len) != NULL) {
    int status = copy_string_without_char(buffer, p_item, str_len, tsep);


Review comment (suggestion):
```suggestion
  output[bytes_written] = '\0';
```

Code after:
  char buffer[PROCESSED_WORD_CAPACITY];
  const size_t str_len = strlen(p);
  if (tsep != '\0' && memchr(p, tsep, str_len) != NULL) {
    const int status = copy_string_without_char(buffer, p, str_len, tsep);
    if (status != 0) {
      // Word is too big, probably will cause an overflow
      *error = ERROR_OVERFLOW;
      return 0;
    }
    p = buffer;
  }

  char *endptr;
  int64_t number = strtoll(p, &endptr, 10);

  if (errno == ERANGE || number > int_max || number < int_min) {
    *error = ERROR_OVERFLOW;
    errno = 0;
    return 0;
  }

  // Skip trailing spaces.
  while (isspace_ascii(*endptr)) {
    ++endptr;
  }

  // Did we use up all the characters?
  if (*endptr) {
    *error = ERROR_INVALID_CHARS;
    return 0;
  }

  *error = 0;
  return number;
}

uint64_t str_to_uint64(uint_state *state, const char *p_item, int64_t int_max,
                       uint64_t uint_max, int *error, char tsep) {
  const char *p = p_item;
  // Skip leading spaces.
  while (isspace_ascii(*p)) {
    ++p;
  }

  // Handle sign.
  if (*p == '-') {
    state->seen_sint = 1;
    *error = 0;
    return 0;
  } else if (*p == '+') {
    p++;
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
  case '-':
    return str[1] != '\0' && isdigit_ascii(str[1]);
  default:
    return false;
  }
}

static inline bool has_only_spaces(const char *str) {
  while (*str != '\0' && isspace_ascii(*str)) {
    str++;
  }
  return *str == '\0';
}

/* Copy a string without `char_to_remove` into `output`.
 */
static int copy_string_without_char(char output[PROCESSED_WORD_CAPACITY],
                                    const char *str, size_t str_len,
                                    char char_to_remove) {
  const char *left = str;
  const char *right;
  const char *end_ptr = str + str_len;
  size_t bytes_written = 0;

  while (left < end_ptr) {
    right = memchr(left, char_to_remove, end_ptr - left);

    // If it doesn't find the char to remove, just copy until EOS.
    size_t chunk_size = right ? right - left : end_ptr - left;

    // check if we have enough space, including the null terminator.
    if (chunk_size + bytes_written >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    // copy block
    memcpy(&output[bytes_written], left, chunk_size);
    bytes_written += chunk_size;

    // Advance past the removed character if we found it.
    left = right ? right + 1 : end_ptr;
  }

  // null terminate
  output[bytes_written] = '\0';
  return 0;
}

int64_t str_to_int64(const char *p_item, int64_t int_min, int64_t int_max,
                     int *error, char tsep) {
  if (!p_item || *p_item == '\0') {
    *error = ERROR_NO_DIGITS;

Review comment (suggestion):
Its really pedantic (this is C, after all...) but ptrdiff_t is typically signed whereas size_t (which memchr expects) is unsigned. Additionally, while most systems will have the same width for these types, I don't know that is universally true.

At the very least this is going to generate a compiler warning, so I think still best to stick with a size_t argument if we can

Code after:

    left = right + 1;
  }

  output[bytes_written] = '\0';
  return 0;
}

int64_t str_to_int64(const char *p_item, int64_t int_min, int64_t int_max,
                     int *error, char tsep) {
  const char *p = p_item;
  // Skip leading spaces.
  while (isspace_ascii(*p)) {
    ++p;
  }

  // Handle sign.
  const bool has_sign = *p == '-' || *p == '+';
  // Handle sign.
  const char *digit_start = has_sign ? p + 1 : p;

  // Check that there is a first digit.
  if (!isdigit_ascii(*digit_start)) {
    // Error...
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  char buffer[PROCESSED_WORD_CAPACITY];
  const size_t str_len = strlen(p);
  if (tsep != '\0' && memchr(p, tsep, str_len) != NULL) {
    const int status = copy_string_without_char(buffer, p, str_len, tsep);
    if (status != 0) {
      // Word is too big, probably will cause an overflow
      *error = ERROR_OVERFLOW;
      return 0;
    }
    p = buffer;
  }

  char *endptr;
  int64_t number = strtoll(p, &endptr, 10);

  if (errno == ERANGE || number > int_max || number < int_min) {
    *error = ERROR_OVERFLOW;
    errno = 0;
    return 0;
  }

  // Skip trailing spaces.
  while (isspace_ascii(*endptr)) {
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
    return str[1] != '\0' && isdigit_ascii(str[1]);
  default:
    return false;
  }
}

static inline bool has_only_spaces(const char *str) {
  while (*str != '\0' && isspace_ascii(*str)) {
    str++;
  }
  return *str == '\0';
}

/* Copy a string without `char_to_remove` into `output`.
 */
static int copy_string_without_char(char output[PROCESSED_WORD_CAPACITY],
                                    const char *str, size_t str_len,
                                    char char_to_remove) {
  const char *left = str;
  const char *right;
  const char *end_ptr = str + str_len;
  size_t bytes_written = 0;
  size_t remaining_bytes_to_read = str_len;

  while (remaining_bytes_to_read > 0) {
    right = (const char *)memchr(left, char_to_remove, remaining_bytes_to_read);

    // If it doesn't find the char to remove, just copy until EOS.
    // We are also casting directly to size_t because
    // `left` never goes beyond `right` or `end_ptr`.
    size_t chunk_size =
        right ? (size_t)(right - left) : (size_t)(end_ptr - left);

    // check if we have enough space, including the null terminator.
    if (chunk_size + bytes_written >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    // copy block
    memcpy(&output[bytes_written], left, chunk_size);
    bytes_written += chunk_size;

    // Advance past the removed character if we found it.
    left = right ? right + 1 : end_ptr;
    remaining_bytes_to_read -= right ? chunk_size + 1 : chunk_size;
  }

  // null terminate
  output[bytes_written] = '\0';
  return 0;
}


Review comment (suggestion):
```suggestion
    right = memchr(left, char_to_remove, remaining_bytes_to_read);
```

While C++ requires this, in C its a bit strange

Code after:
    left = right + 1;
  }

  output[bytes_written] = '\0';
  return 0;
}

int64_t str_to_int64(const char *p_item, int64_t int_min, int64_t int_max,
                     int *error, char tsep) {
  const char *p = p_item;
  // Skip leading spaces.
  while (isspace_ascii(*p)) {
    ++p;
  }

  // Handle sign.
  const bool has_sign = *p == '-' || *p == '+';
  // Handle sign.
  const char *digit_start = has_sign ? p + 1 : p;

  // Check that there is a first digit.
  if (!isdigit_ascii(*digit_start)) {
    // Error...
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  char buffer[PROCESSED_WORD_CAPACITY];
  const size_t str_len = strlen(p);
  if (tsep != '\0' && memchr(p, tsep, str_len) != NULL) {
    const int status = copy_string_without_char(buffer, p, str_len, tsep);
    if (status != 0) {
      // Word is too big, probably will cause an overflow
      *error = ERROR_OVERFLOW;
      return 0;
    }
    p = buffer;
  }

  char *endptr;
  int64_t number = strtoll(p, &endptr, 10);

  if (errno == ERANGE || number > int_max || number < int_min) {
    *error = ERROR_OVERFLOW;
    errno = 0;
    return 0;
  }

  // Skip trailing spaces.
  while (isspace_ascii(*endptr)) {
    ++endptr;
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
static inline bool has_only_spaces(const char *str) {
  while (*str != '\0' && isspace_ascii(*str)) {
    str++;
  }
  return *str == '\0';
}

/* Copy a string without `char_to_remove` into `output`.
 */
static int copy_string_without_char(char output[PROCESSED_WORD_CAPACITY],
                                    const char *str, size_t str_len,
                                    char char_to_remove) {
  const char *left = str;
  const char *right;
  const char *end_ptr = str + str_len;
  size_t bytes_written = 0;
  size_t remaining_bytes_to_read = str_len;

  while (remaining_bytes_to_read > 0) {
    right = (const char *)memchr(left, char_to_remove, remaining_bytes_to_read);

    // If it doesn't find the char to remove, just copy until EOS.
    // We are also casting directly to size_t because
    // `left` never goes beyond `right` or `end_ptr`.
    size_t chunk_size =
        right ? (size_t)(right - left) : (size_t)(end_ptr - left);

    // check if we have enough space, including the null terminator.
    if (chunk_size + bytes_written >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    // copy block
    memcpy(&output[bytes_written], left, chunk_size);
    bytes_written += chunk_size;

    // Advance past the removed character if we found it.
    left = right ? right + 1 : end_ptr;
    remaining_bytes_to_read -= right ? chunk_size + 1 : chunk_size;
  }

  // null terminate
  output[bytes_written] = '\0';
  return 0;
}

int64_t str_to_int64(const char *p_item, int64_t int_min, int64_t int_max,
                     int *error, char tsep) {
  if (!p_item || *p_item == '\0') {
    *error = ERROR_NO_DIGITS;
    return 0;
  }

Review comment (suggestion):
```suggestion
        right ? right - left : end_ptr - left;
```

No point in casting these - the assignment will do it

Code after:

int64_t str_to_int64(const char *p_item, int64_t int_min, int64_t int_max,
                     int *error, char tsep) {
  const char *p = p_item;
  // Skip leading spaces.
  while (isspace_ascii(*p)) {
    ++p;
  }

  // Handle sign.
  const bool has_sign = *p == '-' || *p == '+';
  // Handle sign.
  const char *digit_start = has_sign ? p + 1 : p;

  // Check that there is a first digit.
  if (!isdigit_ascii(*digit_start)) {
    // Error...
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  char buffer[PROCESSED_WORD_CAPACITY];
  const size_t str_len = strlen(p);
  if (tsep != '\0' && memchr(p, tsep, str_len) != NULL) {
    const int status = copy_string_without_char(buffer, p, str_len, tsep);
    if (status != 0) {
      // Word is too big, probably will cause an overflow
      *error = ERROR_OVERFLOW;
      return 0;
    }
    p = buffer;
  }

  char *endptr;
  int64_t number = strtoll(p, &endptr, 10);

  if (errno == ERANGE || number > int_max || number < int_min) {
    *error = ERROR_OVERFLOW;
    errno = 0;
    return 0;
  }

  // Skip trailing spaces.
  while (isspace_ascii(*endptr)) {
    ++endptr;
  }

  // Did we use up all the characters?
  if (*endptr) {
    *error = ERROR_INVALID_CHARS;
    return 0;
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
                                    char char_to_remove) {
  const char *left = str;
  const char *right;
  const char *end_ptr = str + str_len;
  size_t bytes_written = 0;
  size_t remaining_bytes_to_read = str_len;

  while (remaining_bytes_to_read > 0) {
    right = (const char *)memchr(left, char_to_remove, remaining_bytes_to_read);

    // If it doesn't find the char to remove, just copy until EOS.
    // We are also casting directly to size_t because
    // `left` never goes beyond `right` or `end_ptr`.
    size_t chunk_size =
        right ? (size_t)(right - left) : (size_t)(end_ptr - left);

    // check if we have enough space, including the null terminator.
    if (chunk_size + bytes_written >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    // copy block
    memcpy(&output[bytes_written], left, chunk_size);
    bytes_written += chunk_size;

    // Advance past the removed character if we found it.
    left = right ? right + 1 : end_ptr;
    remaining_bytes_to_read -= right ? chunk_size + 1 : chunk_size;
  }

  // null terminate
  output[bytes_written] = '\0';
  return 0;
}

int64_t str_to_int64(const char *p_item, int64_t int_min, int64_t int_max,
                     int *error, char tsep) {
  if (!p_item || *p_item == '\0') {
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  while (isspace_ascii(*p_item)) {
    ++p_item;
  }

  if (!has_digit_int(p_item)) {
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  char buffer[PROCESSED_WORD_CAPACITY];

Review comment (performance):
When in a tight loop you really want to avoid branching; this has 3 different branches to check if `right` is non-NULL - can we consolidate them?

Code after:
  // Handle sign.
  const char *digit_start = has_sign ? p + 1 : p;

  // Check that there is a first digit.
  if (!isdigit_ascii(*digit_start)) {
    // Error...
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  char buffer[PROCESSED_WORD_CAPACITY];
  const size_t str_len = strlen(p);
  if (tsep != '\0' && memchr(p, tsep, str_len) != NULL) {
    const int status = copy_string_without_char(buffer, p, str_len, tsep);
    if (status != 0) {
      // Word is too big, probably will cause an overflow
      *error = ERROR_OVERFLOW;
      return 0;
    }
    p = buffer;
  }

  char *endptr;
  int64_t number = strtoll(p, &endptr, 10);

  if (errno == ERANGE || number > int_max || number < int_min) {
    *error = ERROR_OVERFLOW;
    errno = 0;
    return 0;
  }

  // Skip trailing spaces.
  while (isspace_ascii(*endptr)) {
    ++endptr;
  }

  // Did we use up all the characters?
  if (*endptr) {
    *error = ERROR_INVALID_CHARS;
    return 0;
  }

  *error = 0;
  return number;
}

uint64_t str_to_uint64(uint_state *state, const char *p_item, int64_t int_max,
                       uint64_t uint_max, int *error, char tsep) {
  const char *p = p_item;
  // Skip leading spaces.
  while (isspace_ascii(*p)) {
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
  case '7':
  case '8':
  case '9':
    return true;
  case '+':
  case '-':
    return str[1] != '\0' && isdigit_ascii(str[1]);
  default:
    return false;
  }
}

static inline bool has_only_spaces(const char *str) {
  while (*str != '\0' && isspace_ascii(*str)) {
    str++;
  }
  return *str == '\0';
}

/* Copy a string without `char_to_remove` into `output`.
 */
static int copy_string_without_char(char output[PROCESSED_WORD_CAPACITY],
                                    const char *str, size_t str_len,
                                    char char_to_remove) {
  const char *left = str;
  const char *right;
  const char *end_ptr = str + str_len;
  size_t bytes_written = 0;

  while (left < end_ptr) {
    size_t remaining_bytes_to_read = end_ptr - left;
    right = memchr(left, char_to_remove, remaining_bytes_to_read);

    if (!right) {
      // If it doesn't find the char to remove, just copy until EOS.
      right = end_ptr;
    }

    size_t chunk_size = right - left;

    // check if we have enough space, including the null terminator.
    if (chunk_size + bytes_written >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    // copy block
    memcpy(&output[bytes_written], left, chunk_size);
    bytes_written += chunk_size;

    // Advance past the removed character if we found it.
    left = right + 1;
  }

Review comment (refactor):
Since `right` is only used inside of the loop you can move this therein (to the initial definition)

Code after:
    if (chunk_size + bytes_written >= PROCESSED_WORD_CAPACITY) {
      return -1;
    }
    memcpy(&output[bytes_written], left, chunk_size);
    bytes_written += chunk_size;

    left = right + 1;
  }

  output[bytes_written] = '\0';
  return 0;
}

int64_t str_to_int64(const char *p_item, int64_t int_min, int64_t int_max,
                     int *error, char tsep) {
  const char *p = p_item;
  // Skip leading spaces.
  while (isspace_ascii(*p)) {
    ++p;
  }

  // Handle sign.
  const bool has_sign = *p == '-' || *p == '+';
  // Handle sign.
  const char *digit_start = has_sign ? p + 1 : p;

  // Check that there is a first digit.
  if (!isdigit_ascii(*digit_start)) {
    // Error...
    *error = ERROR_NO_DIGITS;
    return 0;
  }

  char buffer[PROCESSED_WORD_CAPACITY];
  const size_t str_len = strlen(p);
  if (tsep != '\0' && memchr(p, tsep, str_len) != NULL) {
    const int status = copy_string_without_char(buffer, p, str_len, tsep);
    if (status != 0) {
      // Word is too big, probably will cause an overflow
      *error = ERROR_OVERFLOW;
      return 0;
    }
    p = buffer;
  }

  char *endptr;
  int64_t number = strtoll(p, &endptr, 10);

  if (errno == ERANGE || number > int_max || number < int_min) {
    *error = ERROR_OVERFLOW;
    errno = 0;