ronantakizawa/codereview-bench huggingface dataset:
Repository: google/flatbuffers
File: include/flatbuffers/array.h (C/C++)

Code before:
// without endian conversion. Use with care.
// TODO: move these Cast-methods to `internal` namespace.
template<typename T, uint16_t length>
Array<T, length> &CastToArray(T (&arr)[length]) {
  return *reinterpret_cast<Array<T, length> *>(arr);
}

template<typename T, uint16_t length>
const Array<T, length> &CastToArray(const T (&arr)[length]) {
  return *reinterpret_cast<const Array<T, length> *>(arr);
}

template<typename E, typename T, uint16_t length>
Array<E, length> &CastToArrayOfEnum(T (&arr)[length]) {
  static_assert(sizeof(E) == sizeof(T), "invalid enum type E");
  return *reinterpret_cast<Array<E, length> *>(arr);
}

template<typename E, typename T, uint16_t length>
const Array<E, length> &CastToArrayOfEnum(const T (&arr)[length]) {
  static_assert(sizeof(E) == sizeof(T), "invalid enum type E");
  return *reinterpret_cast<const Array<E, length> *>(arr);
}

template<typename T, uint16_t length>
bool operator==(const Array<T, length> &lhs, const Array<T, length> &rhs) {
  return (lhs.size() == rhs.size() &&
          std::memcmp(lhs.Data(), rhs.Data(), rhs.size() * sizeof(T)) == 0);
}

}  // namespace flatbuffers

#endif  // FLATBUFFERS_ARRAY_H_


Review comment (bug):
How about passing in by pointer instead? You will have to nullptr check below, but you can then skip the memcp if the pointers are the same. It will also make the generated code the same, instead of adding the dereferencing part.

Code after:

// Cast a raw T[length] to a raw flatbuffers::Array<T, length>
// without endian conversion. Use with care.
// TODO: move these Cast-methods to `internal` namespace.
template<typename T, uint16_t length>
Array<T, length> &CastToArray(T (&arr)[length]) {
  return *reinterpret_cast<Array<T, length> *>(arr);
}

template<typename T, uint16_t length>
const Array<T, length> &CastToArray(const T (&arr)[length]) {
  return *reinterpret_cast<const Array<T, length> *>(arr);
}

template<typename E, typename T, uint16_t length>
Array<E, length> &CastToArrayOfEnum(T (&arr)[length]) {
  static_assert(sizeof(E) == sizeof(T), "invalid enum type E");
  return *reinterpret_cast<Array<E, length> *>(arr);
}

template<typename E, typename T, uint16_t length>
const Array<E, length> &CastToArrayOfEnum(const T (&arr)[length]) {
  static_assert(sizeof(E) == sizeof(T), "invalid enum type E");
  return *reinterpret_cast<const Array<E, length> *>(arr);
}

template<typename T, uint16_t length>
bool operator==(const Array<T, length> &lhs,
                const Array<T, length> &rhs) noexcept {
  return std::addressof(lhs) == std::addressof(rhs) ||
         (lhs.size() == rhs.size() &&
          std::memcmp(lhs.Data(), rhs.Data(), rhs.size() * sizeof(T)) == 0);
}

}  // namespace flatbuffers

#endif  // FLATBUFFERS_ARRAY_H_
Repository: google/flatbuffers
File: src/file_name_saving_file_manager.cpp (C++)

Code before:
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

#include <string>
#include <set>
#include <fstream>

#include "flatbuffers/file_manager.h"

namespace flatbuffers {

class FileNameSavingFileManagerManager : public FileManager {
 public:
  bool SaveFile(const std::string &absolute_file_name, const std::string &content) override {
    (void)content;
    auto pair = file_names_.insert(absolute_file_name);
    return pair.second;
  }

  bool ReadFile(const std::string &absolute_file_name, std::string * content) override {
    (void) absolute_file_name;
    (void) content;
    return false;
  }

  //std::set<std::string> FileNameSavingFileManagers() { return file_names_; }
};

}  // namespace flatbuffers


Review comment (suggestion):
i believe you can just not name the `content` in the signature and then you don't have to worry about this.

Code after:
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

#include <fstream>
#include <set>
#include <string>

#include "flatbuffers/file_manager.h"

namespace flatbuffers {

class FileNameSavingFileManager : public FileManager {
 public:
  FileNameSavingFileManager(std::set<std::string> file_names)
      : file_names_(file_names) {}

  bool SaveFile(const std::string &absolute_file_name,
                const std::string &content) override {
    (void)content;
    auto pair = file_names_.insert(absolute_file_name);
    // pair.second indicates whether the insertion is
    // successful or not.
    return pair.second;
  }

  bool Loadfile(const std::string &absolute_file_name, std::string *content) {
    (void) absolute_file_name;
    (void) content;
    return false;
  }

 private:
  std::set<std::string> file_names_;
};

}  // namespace flatbuffers
Repository: google/flatbuffers
File: src/util.cpp (C++)

Code before:
}

std::string PosixPath(const char *path) {
  std::string p = path;
  std::replace(p.begin(), p.end(), '\\', '/');
  return p;
}
std::string PosixPath(const std::string &path) {
  return PosixPath(path.c_str());
}

void EnsureDirExists(const std::string &filepath) {
  auto parent = StripFileName(filepath);
  if (parent.length()) EnsureDirExists(parent);
    // clang-format off

  #ifdef _WIN32
    (void)_mkdir(filepath.c_str());
  #else
    mkdir(filepath.c_str(), S_IRWXU|S_IRGRP|S_IXGRP);
  #endif
  // clang-format on
}

std::string FilePath(const std::string& project, const std::string& filePath, bool absolute) {
    if (absolute) {
      return AbsolutePath(filePath);
    } else {
      return RelativeToRootPath(project, filePath);
    }
}

std::string AbsolutePath(const std::string &filepath) {
  // clang-format off

  #ifdef FLATBUFFERS_NO_ABSOLUTE_PATH_RESOLUTION
    return filepath;
  #else
    #if defined(_WIN32) || defined(__MINGW32__) || defined(__MINGW64__) || defined(__CYGWIN__)
      char abs_path[MAX_PATH];
      return GetFullPathNameA(filepath.c_str(), MAX_PATH, abs_path, nullptr)
    #else
      char *abs_path_temp = realpath(filepath.c_str(), nullptr);
      bool success = abs_path_temp != nullptr;
      std::string abs_path;
      if(success) {
        abs_path = abs_path_temp;
        free(abs_path_temp);
      }
      return success
    #endif

Review comment (suggestion):
prefer ternary style:

```return (absolute) ? AbsolutePath(filePath) : RelativeToRootPath(project, filePath);```

Code after:
}

std::string PosixPath(const char *path) {
  std::string p = path;
  std::replace(p.begin(), p.end(), '\\', '/');
  return p;
}
std::string PosixPath(const std::string &path) {
  return PosixPath(path.c_str());
}

void EnsureDirExists(const std::string &filepath) {
  auto parent = StripFileName(filepath);
  if (parent.length()) EnsureDirExists(parent);
    // clang-format off

  #ifdef _WIN32
    (void)_mkdir(filepath.c_str());
  #else
    mkdir(filepath.c_str(), S_IRWXU|S_IRGRP|S_IXGRP);
  #endif
  // clang-format on
}

std::string FilePath(const std::string& project, const std::string& filePath, bool absolute) {
    return (absolute) ? AbsolutePath(filePath) : RelativeToRootPath(project, filePath);
}

std::string AbsolutePath(const std::string &filepath) {
  // clang-format off

  #ifdef FLATBUFFERS_NO_ABSOLUTE_PATH_RESOLUTION
    return filepath;
  #else
    #if defined(_WIN32) || defined(__MINGW32__) || defined(__MINGW64__) || defined(__CYGWIN__)
      char abs_path[MAX_PATH];
      return GetFullPathNameA(filepath.c_str(), MAX_PATH, abs_path, nullptr)
    #else
      char *abs_path_temp = realpath(filepath.c_str(), nullptr);
      bool success = abs_path_temp != nullptr;
      std::string abs_path;
      if(success) {
        abs_path = abs_path_temp;
        free(abs_path_temp);
      }
      return success
    #endif
      ? abs_path
      : filepath;
  #endif // FLATBUFFERS_NO_ABSOLUTE_PATH_RESOLUTION
  // clang-format on
Repository: google/flatbuffers
File: lua/flatbuffers/builder.lua (Lua)

Code before:

    return self:EndVector(l)
end

function mt:Slot(slotnum)
    assert(self.nested)
    -- n.b. slot number is 0-based
    self.currentVTable[slotnum + 1] = self:Offset()
end

local function finish(self, rootTable, sizePrefix, fileIdentifier)
    UOffsetT:EnforceNumber(rootTable)
    local hasFid = fileIdentifier ~= nil
    if hasFid then
        assert(#fileIdentifier == 4, "File identifier must be exactly 4 bytes")
    end

    -- alignment:
    -- root offset (4)
    -- optional file id (4)
    -- optional size prefix (4)
    self:Prep(
        self.minalign,
        (sizePrefix and 4 or 0) +
        4 +
        (hasFid and 4 or 0)
    )

    -- root offset (points past file identifier if present)
    self:PrependUOffsetTRelative(rootTable + (hasFid and 4 or 0))

    -- file identifier
    if hasFid then
        for i = 4, 1, -1 do
            self:PrependByte(string.byte(fileIdentifier, i))
        end
    end

    -- size prefix
    if sizePrefix then
        local size = self.bytes.size - self.head
        Int32:EnforceNumber(size)
        self:PrependInt32(size)
    end

    self.finished = true
    return self.head
end


function mt:Finish(rootTable)

Review comment (performance):
This is a slow language that likely does not do common subexpression elimination, so wouldn't do `(hasFid and 4 or 0)` twice. In fact, maybe instead of `hasFid` use `sizeFid` which can have values 0 or 4 and can function both as bool and as value.

Code after:

    return self:EndVector(l)
end

function mt:Slot(slotnum)
    assert(self.nested)
    -- n.b. slot number is 0-based
    self.currentVTable[slotnum + 1] = self:Offset()
end

local function finish(self, rootTable, sizePrefix, fileIdentifier)
    UOffsetT:EnforceNumber(rootTable)
    local hasFid = fileIdentifier ~= nil
    if hasFid then
        assert(#fileIdentifier == 4, "File identifier must be exactly 4 bytes")
    end

    local fid_byte_count = (hasFid and 4 or 0)

    -- alignment:
    -- root offset (4)
    -- optional file id (4)
    -- optional size prefix (4)
    self:Prep(
        self.minalign,
        (sizePrefix and 4 or 0) +
        4 +
        fid_byte_count
    )

    -- root offset (points past file identifier if present)
    self:PrependUOffsetTRelative(rootTable + fid_byte_count)

    -- file identifier
    if hasFid then
        for i = 4, 1, -1 do
            self:PrependByte(string.byte(fileIdentifier, i))
        end
    end

    -- size prefix
    if sizePrefix then
        local size = self.bytes.size - self.head
        Int32:EnforceNumber(size)
        self:PrependInt32(size)
    end

    self.finished = true
    return self.head
end
Repository: google/googletest
File: googletest/include/gtest/gtest-printers.h (C/C++)

Code before:
template <typename T>
class UniversalPrinter<Optional<T>> {
 public:
  static void Print(const Optional<T>& value, ::std::ostream* os) {
    *os << '(';
    if (!value) {
      *os << "nullopt";
    } else {
      UniversalPrint(*value, os);
    }
    *os << ')';
  }
};

#endif  // GTEST_INTERNAL_HAS_OPTIONAL

#if GTEST_INTERNAL_HAS_VARIANT

// Printer for std::variant / absl::variant

template <typename... T>
class UniversalPrinter<Variant<T...>> {
 public:
  static void Print(const Variant<T...>& value, ::std::ostream* os) {
    *os << '(';
#if __cplusplus >= 201703L
    std::visit(Visitor{os}, value);
#else
    absl::visit(Visitor{os}, value);
#endif  // __cplusplus >= 201703L
    *os << ')';
  }

 private:
  struct Visitor {
    template <typename U>
    void operator()(const U& u) const {
      *os << "'" << GetTypeName<U>() << "' with value ";
      UniversalPrint(u, os);
    }
    ::std::ostream* os;
  };
};

#endif  // GTEST_INTERNAL_HAS_VARIANT

// UniversalPrintArray(begin, len, os) prints an array of 'len'
// elements, starting at address 'begin'.
template <typename T>
void UniversalPrintArray(const T* begin, size_t len, ::std::ostream* os) {
  if (len == 0) {

Review comment (suggestion):
I think we should check `GTEST_HAS_ABSL` instead of `__cplusplus` to be consistent with "gtest-port.h", e.g. `#if GTEST_HAS_ABSL // use absl::visit(...)`.
https://github.com/abseil/abseil-cpp/blob/7853a7586c492ce8905c9e49f8679dada6354f2c/absl/base/config.h#L581

Code after:
template <typename T>
class UniversalPrinter<Optional<T>> {
 public:
  static void Print(const Optional<T>& value, ::std::ostream* os) {
    *os << '(';
    if (!value) {
      *os << "nullopt";
    } else {
      UniversalPrint(*value, os);
    }
    *os << ')';
  }
};

#endif  // GTEST_INTERNAL_HAS_OPTIONAL

#if GTEST_INTERNAL_HAS_VARIANT

// Printer for std::variant / absl::variant

template <typename... T>
class UniversalPrinter<Variant<T...>> {
 public:
  static void Print(const Variant<T...>& value, ::std::ostream* os) {
    *os << '(';
#if GTEST_HAS_ABSL
    absl::visit(Visitor{os, value.index()}, value);
#else
    std::visit(Visitor{os, value.index()}, value);
#endif  // GTEST_HAS_ABSL
    *os << ')';
  }

 private:
  struct Visitor {
    template <typename U>
    void operator()(const U& u) const {
      *os << "'" << GetTypeName<U>() << "(" << index << ")' with value ";
      UniversalPrint(u, os);
    }
    ::std::ostream* os;
    std::size_t index;
  };
};

#endif  // GTEST_INTERNAL_HAS_VARIANT

// UniversalPrintArray(begin, len, os) prints an array of 'len'
// elements, starting at address 'begin'.
template <typename T>
void UniversalPrintArray(const T* begin, size_t len, ::std::ostream* os) {
Repository: google/python-fire
File: fire/testutils.py (Python)

Code before:
    for name, regexp, fp in [('stdout', stdout, stdout_fp),
                             ('stderr', stderr, stderr_fp)]:
      value = fp.getvalue()
      if regexp is None:
        if value:
          raise AssertionError('%s: Expected no output. Got: %r' %
                               (name, value))
      else:
        if not re.search(regexp, value, re.DOTALL | re.MULTILINE):
          raise AssertionError('%s: Expected %r to match %r' %
                               (name, value, regexp))

  @contextlib.contextmanager
  def assertRaisesFireExit(self, code, regexp=None):
    """Asserts that a FireExit error is raised in the context.

    Allows tests to check that Fire's wrapper around SystemExit is raised
    and that a regexp is matched in the output.

    Args:
      code: The status code that the FireExit should contain.
      regexp: stdout must match this regex.
    Yields:
      Yields to the wrapped context.
    """
    if regexp is None:
      regexp = '.*'
    with self.assertOutputMatches(stderr=regexp):
      with self.assertRaises(core.FireExit):
        try:
          yield
        except core.FireExit as exc:
          if exc.code != code:
            raise AssertionError('Incorrect exit code: %r != %r' % (exc.code,
                                                                    code))
          self.assertIsInstance(exc.trace, trace.FireTrace)
          raise


def main():
  unittest.main()


Review comment (suggestion):
as long as you're in here, this can be the default value to the fn.
`def assertRaisesFireExit(self, code, regexp='.*'):`

Code after:
        sys.stderr.write(stderr_fp.getvalue())

    for name, regexp, fp in [('stdout', stdout, stdout_fp),
                             ('stderr', stderr, stderr_fp)]:
      value = fp.getvalue()
      if regexp is None:
        if value:
          raise AssertionError('%s: Expected no output. Got: %r' %
                               (name, value))
      else:
        if not re.search(regexp, value, re.DOTALL | re.MULTILINE):
          raise AssertionError('%s: Expected %r to match %r' %
                               (name, value, regexp))

  @contextlib.contextmanager
  def assertRaisesFireExit(self, code, regexp='.*'):
    """Asserts that a FireExit error is raised in the context.

    Allows tests to check that Fire's wrapper around SystemExit is raised
    and that a regexp is matched in the output.

    Args:
      code: The status code that the FireExit should contain.
      regexp: stdout must match this regex.
    Yields:
      Yields to the wrapped context.
    """
    with self.assertOutputMatches(stderr=regexp):
      with self.assertRaises(core.FireExit):
        try:
          yield
        except core.FireExit as exc:
          if exc.code != code:
            raise AssertionError('Incorrect exit code: %r != %r' % (exc.code,
                                                                    code))
          self.assertIsInstance(exc.trace, trace.FireTrace)
          raise


def main():
  unittest.main()
Repository: haydenbleasel/next-forge
File: packages/notifications/components/provider.tsx (TypeScript)

Code before:
'use client';

import { KnockFeedProvider, KnockProvider } from '@knocklabs/react';
import { useTheme } from 'next-themes';
import type { ReactNode } from 'react';
import { keys } from '../keys';

const knockApiKey = keys().NEXT_PUBLIC_KNOCK_API_KEY;
const knockFeedChannelId = keys().NEXT_PUBLIC_KNOCK_FEED_CHANNEL_ID;

type NotificationsProviderProps = {
  children: ReactNode;
  userId: string;
};

export const NotificationsProvider = ({
  children,
  userId,
}: NotificationsProviderProps) => {
  if (!knockApiKey || !knockFeedChannelId) {
    return children;
  }

  const { theme: rawTheme } = useTheme();
  const theme = rawTheme === 'dark' ? 'dark' : 'light';

  return (
    <KnockProvider apiKey={knockApiKey} userId={userId}>
      <KnockFeedProvider feedId={knockFeedChannelId} colorMode={theme}>
        {children}
      </KnockFeedProvider>
    </KnockProvider>
  );
};


Review comment (suggestion):
```TSX
const { resolvedTheme } = useTheme(); // Returns same as theme value but resolve system to the corresponding value
```

useTheme in [next-themes api doc](https://github.com/pacocoursey/next-themes?tab=readme-ov-file#usetheme-1)

Code after:
'use client';

import { ColorMode, KnockFeedProvider, KnockProvider } from '@knocklabs/react';
import type { ReactNode } from 'react';
import { keys } from '../keys';

const knockApiKey = keys().NEXT_PUBLIC_KNOCK_API_KEY;
const knockFeedChannelId = keys().NEXT_PUBLIC_KNOCK_FEED_CHANNEL_ID;

type NotificationsProviderProps = {
  children: ReactNode;
  userId: string;
  theme: ColorMode;
};

export const NotificationsProvider = ({
  children,
  theme,
  userId,
}: NotificationsProviderProps) => {
  if (!knockApiKey || !knockFeedChannelId) {
    return children;
  }

  return (
    <KnockProvider apiKey={knockApiKey} userId={userId}>
      <KnockFeedProvider feedId={knockFeedChannelId} colorMode={theme}>
        {children}
      </KnockFeedProvider>
    </KnockProvider>
  );
};
Repository: humanlayer/humanlayer
File: claudecode-go/client.go (Go)

Code before:
				}
			} else if dir == "~" {
				if home, err := os.UserHomeDir(); err == nil {
					expandedDir = home
				}
			}

			// Convert to absolute path
			absPath, err := filepath.Abs(expandedDir)
			if err == nil {
				log.Printf("Adding directory (expanded): %s -> %s", dir, absPath)
				args = append(args, "--add-dir", absPath)
			} else {
				// Fallback to original if absolute path conversion fails
				log.Printf("Adding directory (original, expansion failed): %s", dir)
				args = append(args, "--add-dir", dir)
			}
		}
	} else {
		log.Printf("No additional directories to add")
	}

	// Verbose
	if config.Verbose {
		args = append(args, "--verbose")
	}

	return args, nil
}

// Launch starts a new Claude session and returns immediately
func (c *Client) Launch(config SessionConfig) (*Session, error) {
	args, err := c.buildArgs(config)
	if err != nil {
		return nil, err
	}

	log.Printf("Executing Claude command: %s %v", c.claudePath, args)
	cmd := exec.Command(c.claudePath, args...)

	// Set environment variables if specified
	if len(config.Env) > 0 {
		cmd.Env = os.Environ() // Start with current environment
		for key, value := range config.Env {
			cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", key, value))
		}
	}

	// Set working directory if specified
	if config.WorkingDir != "" {
		workingDir := config.WorkingDir

Review comment (bug):
i mean technical if `config.Query == ""` then we have nothing to do and claude will fail but...the old code did that so fine with this

Code after:
			}

			// Convert to absolute path
			absPath, err := filepath.Abs(expandedDir)
			if err == nil {
				log.Printf("Adding directory (expanded): %s -> %s", dir, absPath)
				args = append(args, "--add-dir", absPath)
			} else {
				// Fallback to original if absolute path conversion fails
				log.Printf("Adding directory (original, expansion failed): %s", dir)
				args = append(args, "--add-dir", dir)
			}
		}
	} else {
		log.Printf("No additional directories to add")
	}

	// Verbose
	if config.Verbose {
		args = append(args, "--verbose")
	}

	// Always use print mode for SDK - MUST be the last flag before --
	// The -- separator tells the CLI parser to stop interpreting flags
	// Correct order: <all flags> --print -- <query>
	if config.Query != "" {
		args = append(args, "--print")
		args = append(args, "--")
		args = append(args, config.Query)
	}

	return args, nil
}

// Launch starts a new Claude session and returns immediately
func (c *Client) Launch(config SessionConfig) (*Session, error) {
	args, err := c.buildArgs(config)
	if err != nil {
		return nil, err
	}

	log.Printf("Executing Claude command: %s %v", c.claudePath, args)
	cmd := exec.Command(c.claudePath, args...)

	// Set environment variables if specified
	if len(config.Env) > 0 {
		cmd.Env = os.Environ() // Start with current environment
		for key, value := range config.Env {
			cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", key, value))
		}
	}