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

Code before:
  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) {
    // Python's integers can handle pure overflow errors,
    // but for invalid characters, try using different conversion methods.
    *error = *endptr ? ERROR_INVALID_CHARS : 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, int *error,
                       char tsep) {
  const char *p = p_item;
  // Skip leading spaces.

Review comment (suggestion):
Are you sure that `strtoll` sets `ERANGE` as the error when there are invalid characters? The man page for it appears to suggest otherwise:

```
ERRORS
       This function does not modify errno on success.

       EINVAL (not in C99) The given base contains an unsupported value.

       ERANGE The resulting value was out of range.

       The implementation may also set errno to EINVAL in  case  no  conversion
       was performed (no digits seen, and 0 returned).
```

Code after:
  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) {
    *error = *endptr ? ERROR_INVALID_CHARS : 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, int *error,
                       char tsep) {
  const char *p = p_item;
  // Skip leading spaces.
  while (isspace_ascii(*p)) {
    ++p;
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
  case '-':
    return isdigit_ascii(str[1]);
  default:
    return 0;
  }
}

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

/* Copy a string without `char_to_remove`.
 * The returned memory should be free-d with a call to `free`.
 */
static char *copy_string_without_char(const char *str, char char_to_remove) {
  size_t chars_to_copy = 0;
  for (const char *src = str; *src != '\0'; src++) {
    if (*src != char_to_remove) {
      chars_to_copy++;
    }
  }

  char *start = malloc((chars_to_copy + 1) * sizeof(char));
  if (!start) {
    return NULL;
  }

  char *dst = start;
  for (const char *src = str; *src != '\0'; src++) {
    if (*src != char_to_remove) {
      *dst++ = *src;
    }
  }
  *dst = '\0';

  return start;
}

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;
  }

Review comment (performance):
For performance reasons we should avoid using the heap as much as possible. Given this only applies to numeric digits, I think it would be better to just stack allocate an array of a certain size. 

Trying to be forward looking, we can probably get away with a local heap of size 128 bytes, since that covers the maximum character length of Arrow's Decimal256 (76), and sizes up to a power of 2. Anything longer that that can reasonably not be parsed as numeric

Code after:
      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;
    return 0;
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 int has_digit_int(const char *str) {
  if (!str || *str == '\0') {
    return 0;
  }

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

static inline int has_only_spaces(const char *str) {

Review comment (suggestion):
```suggestion
static inline bool has_digit_int(const char *str) {
```

We are far enough removed from C89 that we can use the bool type :-)

Code after:
    while (isspace_ascii(**q)) {
      (*q)++;
    }
  }
  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:
  case '4':
  case '5':
  case '6':
  case '7':
  case '8':
  case '9':
    return 1;
  case '+':
  case '-':
    return isdigit_ascii(str[1]);
  default:
    return 0;
  }
}

static inline int 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`,
 * while ensuring it's null terminated.
 */
static void copy_string_without_char(char *output, const char *str,
                                     char char_to_remove, size_t output_size) {
  size_t i = 0;
  for (const char *src = str; *src != '\0' && i < output_size; src++) {
    if (*src != char_to_remove) {
      output[i++] = *src;
    }
  }
  if (i < output_size) {
    output[i] = '\0';
  } else {
    // str is too big, probably would overflow
    errno = ERANGE;
  }
}

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;
  }

Review comment (suggestion):
```suggestion
static void copy_string_without_char(char output[PROCESSED_WORD_CAPACITY], const char *str,
```

Don't think you need to pass this capacity as a separate argument, since it is always the same value

Code after:
      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;
}

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;
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
  case '-':
    return isdigit_ascii(str[1]);
  default:
    return 0;
  }
}

static inline int 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`,
 * while ensuring it's null terminated.
 */
static void copy_string_without_char(char *output, const char *str,
                                     char char_to_remove, size_t output_size) {
  size_t i = 0;
  for (const char *src = str; *src != '\0' && i < output_size; src++) {
    if (*src != char_to_remove) {
      output[i++] = *src;
    }
  }
  if (i < output_size) {
    output[i] = '\0';
  } else {
    // str is too big, probably would overflow
    errno = ERANGE;
  }
}

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;
  if (tsep != '\0' && strchr(p_item, tsep) != NULL) {

Review comment (suggestion):
This is pretty wasteful to continually write null bytes. You could either memset the buffer before you send it, or ideally short circuit when you've processed all the necessary bytes

Code after:
    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;
    return 0;
  }
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
  case '7':
  case '8':
  case '9':
    return 1;
  case '+':
  case '-':
    return isdigit_ascii(str[1]);
  default:
    return 0;
  }
}

static inline int 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`,
 * while ensuring it's null terminated.
 */
static void copy_string_without_char(char *output, const char *str,
                                     char char_to_remove, size_t output_size) {
  size_t i = 0;
  for (const char *src = str; *src != '\0' && i < output_size; src++) {
    if (*src != char_to_remove) {
      output[i++] = *src;
    }
  }
  if (i < output_size) {
    output[i] = '\0';
  } else {
    // str is too big, probably would overflow
    errno = ERANGE;
  }
}

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;

Review comment (suggestion):
It would be great to avoid character-by-character writes if we can, especially since this is a relatively sparse character to search for.

You might be limited with what you can do without some of the larger changes I suggested, but maybe even just finding a region and writing multiple bytes at once will be more performant. 

Code after:
    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;
}

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) {
Repository: pandas-dev/pandas
File: pandas/_libs/src/parser/tokenizer.c (C)

Code before:
  return *str == '\0';
}

static int power_int(int base, int exponent) {
  // https://en.wikipedia.org/wiki/Exponentiation_by_squaring
  if (exponent == 0) {
    return 1;
  } else if (exponent < 0) {
    return 0;
  }

  int result = 1;

  while (exponent > 1) {
    if (exponent % 2 == 1) {
      result *= base;
      exponent--;
    }
    result *= result;
    exponent /= 2;
  }

  return result * base;
}

static inline int64_t add_int_check_overflow(int64_t lhs, int64_t rhs,
                                             int64_t mul_lhs) {
  // rhs will always be positive, because this function
  // only executes after the first parse, hence the sign will always go to lhs.
  // if lhs > 0:
  // Will overflow if (mul_lhs * lhs) + rhs > INT_MAX
  // iff lhs > (INT_MAX - rhs) / mul_lhs
  // if lhs < 0:
  // Will underflow if (mul_lhs * lhs) - rhs < INT_MIN
  // iff lhs < (INT_MIN + rhs) / mul_lhs
  if (lhs >= 0) {
    if (lhs > (INT_MAX - rhs) / mul_lhs) {
      errno = ERANGE;
    }
  } else {
    if (lhs < (INT_MIN + rhs) / mul_lhs) {
      errno = ERANGE;
    }
    rhs = -rhs;
  }
  return lhs * mul_lhs + rhs;
}

static inline uint64_t add_uint_check_overflow(uint64_t lhs, uint64_t rhs,
                                               uint64_t mul_lhs) {
  if (lhs > (UINT_MAX - rhs) / mul_lhs) {

Review comment (bug):
There's no need to implement custom overflow detection. We already have macros that defer to compiler builtins (see the np_datetime.c module). You can refactor those if needed to make available here. 

FWIW C23 also standardizes checked overflow functions. I'm not sure how far along MSVC is in implementing that, but for other platforms we can likely use the standard on newer compilers. 

Code after:
                     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:
  }

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

  errno = 0;
  char *endptr = NULL;
  int64_t result = strtoll(p_item, &endptr, 10);
  bool is_negative = result < 0;

  while (errno == 0 && tsep != '\0' && *endptr == tsep) {
    // Skip multiple consecutive tsep
    while (*endptr == tsep) {
      endptr++;
    }

    char *new_end = NULL;
    int64_t next_part = strtoll(endptr, &new_end, 10);
    if (is_negative) {
      next_part = -next_part;
    }

    ptrdiff_t digits = new_end - endptr;
    int64_t mul_result = power_int(10, (int)digits);
    // result * mul_result
    if (checked_int64_mul(result, mul_result, &result)) {
      // overflow
      errno = ERANGE;
    }
    // result + next_part
    if (checked_int64_add(result, next_part, &result)) {
      // overflow
      errno = ERANGE;
    }
    endptr = new_end;
  }

  if (!has_only_spaces(endptr)) {
    // Check first for invalid characters because we may
    // want to skip integer parsing if we find one.
    *error = ERROR_INVALID_CHARS;
    result = 0;
  } else if (errno == ERANGE || result > int_max || result < int_min) {
    *error = ERROR_OVERFLOW;
    result = 0;
  } else {
    *error = 0;
  }


Review comment (suggestion):
Ah OK - I think you are assembling this in pieces. However, I'd still suggest just getting rid of the separators in a local buffer and then just calling `strtoll` (or whatever function) on the whole buffer. There's a lot of nuance to range/value handling that its not worth reimplementing

Code after:
  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++;
  }

  // Check that there is a first digit.
  if (!isdigit_ascii(*p)) {
    // 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