ronantakizawa/codereview-bench huggingface dataset:
Repository: PathOfBuildingCommunity/PathOfBuilding
File: src/Classes/ModList.lua (Lua)

Code before:
	t_insert(self, mod)
end

---ReplaceModInternal
---  Replaces an existing matching mod with a new mod.
---  If no matching mod exists, then the function returns false
---@param mod table
---@return boolean @Whether any mod was replaced
function ModListClass:ReplaceModInternal(mod)
	-- Find the index of the existing mod, if it is in the table
	for i, curMod in ipairs(self) do
		if mod.name == curMod.name and mod.type == curMod.type and mod.flags == curMod.flags and mod.keywordFlags == curMod.keywordFlags and mod.source == curMod.source then
			self[i] = mod
			return true
		end
	end

	if self.parent then
		return self.parent:ReplaceModInternal(mod)
	end
	
	return false
end

function ModListClass:MergeMod(mod)
	if mod.type == "BASE" or mod.type == "INC" then
		for i = 1, #self do
			if modLib.compareModParams(self[i], mod) then
				self[i] = copyTable(self[i], true)
				self[i].value = self[i].value + mod.value
				return
			end
		end
	end
	self:AddMod(mod)
end

function ModListClass:AddList(modList)
	if modList then
		for i = 1, #modList do
			t_insert(self, modList[i])
		end
	end
end

function ModListClass:MergeNewMod(...)
	self:MergeMod(mod_createMod(...))
end


function ModListClass:SumInternal(context, modType, cfg, flags, keywordFlags, source, ...)

Review comment (suggestion):
I felt this was safe because the only places we merge mods are places that don't even have MORE modifiers, let alone multiple.  If we end up being able to scale an item that has 2+ more multipliers of the same type though, we should revisit this to make sure it's accurate

Code after:
	t_insert(self, mod)
end

---ReplaceModInternal
---  Replaces an existing matching mod with a new mod.
---  If no matching mod exists, then the function returns false
---@param mod table
---@return boolean @Whether any mod was replaced
function ModListClass:ReplaceModInternal(mod)
	-- Find the index of the existing mod, if it is in the table
	for i, curMod in ipairs(self) do
		if mod.name == curMod.name and mod.type == curMod.type and mod.flags == curMod.flags and mod.keywordFlags == curMod.keywordFlags and mod.source == curMod.source then
			self[i] = mod
			return true
		end
	end

	if self.parent then
		return self.parent:ReplaceModInternal(mod)
	end
	
	return false
end

function ModListClass:MergeMod(mod)
	if mod.type == "BASE" or mod.type == "INC" or mod.type == "MORE" then
		for i = 1, #self do
			if modLib.compareModParams(self[i], mod) then
				self[i] = copyTable(self[i], true)
				self[i].value = self[i].value + mod.value
				return
			end
		end
	end
	self:AddMod(mod)
end

function ModListClass:AddList(modList)
	if modList then
		for i = 1, #modList do
			t_insert(self, modList[i])
		end
	end
end

function ModListClass:MergeNewMod(...)
	self:MergeMod(mod_createMod(...))
end


function ModListClass:SumInternal(context, modType, cfg, flags, keywordFlags, source, ...)
Repository: PrefectHQ/prefect
File: src/integrations/prefect-aws/prefect_aws/plugins.py (Python)

Code before:
from __future__ import annotations

import ssl
from typing import Any, Mapping

import boto3
import sqlalchemy as sa

from prefect._experimental.plugins import hookimpl


@hookimpl
def get_database_connection_params(
    connection_url: str, settings: Any
) -> Mapping[str, Any]:
    iam_settings = settings.server.database.sqlalchemy.connect_args.iam

    if not iam_settings.enabled:
        return {}

    url = sa.engine.make_url(connection_url)
    connect_args = {}

    def get_iam_token() -> str:
        session = boto3.Session()
        region = iam_settings.region_name or session.region_name
        client = session.client("rds", region_name=region)
        token = client.generate_db_auth_token(
            DBHostname=url.host,
            Port=url.port or 5432,
            DBUsername=url.username,
            Region=region,
        )
        return token

    # IAM authentication requires SSL
    # Use PROTOCOL_TLS_CLIENT to avoid loading default system certs

Review comment (suggestion):
This should use the existing `@register_hook` decorator.
```suggestion
@register_hook
```

Code after:
from __future__ import annotations

import ssl
from typing import Any, Mapping

import boto3
import sqlalchemy as sa

from prefect._experimental.plugins import register_hook
from prefect_aws.settings import AwsSettings


@register_hook
def set_database_connection_params(
    connection_url: str, settings: Any
) -> Mapping[str, Any]:
    iam_settings = AwsSettings().rds.iam

    if not iam_settings.enabled:
        return {}

    url = sa.engine.make_url(connection_url)
    connect_args = {}

    def get_iam_token() -> str:
        session = boto3.Session()
        region = iam_settings.region_name or session.region_name
        client = session.client("rds", region_name=region)
        token = client.generate_db_auth_token(
            DBHostname=url.host,
            Port=url.port or 5432,
            DBUsername=url.username,
            Region=region,
        )
        return token

    # IAM authentication requires SSL
Repository: PrefectHQ/prefect
File: src/prefect/_internal/compatibility/backports.py (Python)

Code before:
"""Functionality we want to use from later Python versions."""

import sys

try:
    import tomllib  # 3.11+
except ImportError:
    import toml as tomllib  # fallback on Python <3.11

    tomllib.TOMLDecodeError = tomllib.TomlDecodeError

# enable importing tomllib from backports as a module (like os.path)
sys.modules["prefect._internal.compatibility.backports.tomllib"] = tomllib

__all__ = ["tomllib"]


Review comment (refactor):
I'd rather not make modifications to `sys.modules` like. Can you remove this line and update the imports to this:
```python
from prefect._internal.compatibility.backports import tomllib
```

Code after:
"""Functionality we want to use from later Python versions."""

try:
    import tomllib  # 3.11+
except ImportError:
    import toml as tomllib  # fallback on Python <3.11

    tomllib.TOMLDecodeError = tomllib.TomlDecodeError


__all__ = ["tomllib"]
Repository: ReactiveX/RxJava
File: src/main/java/io/reactivex/rxjava3/internal/operators/observable/ObservableCache.java (Java)

Code before:
     * Returns true if there are observers subscribed to this observable.
     * @return true if the cache has observers
     */
    /* public */ boolean hasObservers() {
        return multicaster.observers.get().length != 0;
    }

    /**
     * Returns the number of events currently cached.
     * @return the number of currently cached event count
     */
    /* public */ long cachedEventCount() {
        return multicaster.size;
    }

    static final class Multicaster<T> implements Observer<T> {

        /**
         * The number of items per cached nodes.
         */
        final int capacityHint;

        /**
         * The current known array of observer state to notify.
         */
        final AtomicReference<CacheDisposable<T>[]> observers;

        /**
         * The total number of elements in the list available for reads.
         */
        volatile long size;

        /**
         * The current tail of the linked structure holding the items.
         */
        Node<T> tail;

        /**
         * How many items have been put into the tail node so far.
         */
        int tailOffset;

        /**
         * If {@link #observers} is {@link #TERMINATED}, this holds the terminal error if not null.
         */
        Throwable error;

        /**
         * True if the source has terminated.
         */
        volatile boolean done;

Review comment (performance):
Please inline this into the class:
```java
static final class Multicaster<T> extends AtomicReference<CacheDisposable<T>[]> implements Observer<T>
```

Code after:
     * Returns true if there are observers subscribed to this observable.
     * @return true if the cache has observers
     */
    /* public */ boolean hasObservers() {
        return multicaster.get().length != 0;
    }

    /**
     * Returns the number of events currently cached.
     * @return the number of currently cached event count
     */
    /* public */ long cachedEventCount() {
        return multicaster.size;
    }

    static final class Multicaster<T> extends AtomicReference<CacheDisposable<T>[]> implements Observer<T> {

        /**
         * The number of items per cached nodes.
         */
        final int capacityHint;

        /**
         * The total number of elements in the list available for reads.
         */
        volatile long size;

        /**
         * The current tail of the linked structure holding the items.
         */
        Node<T> tail;

        /**
         * How many items have been put into the tail node so far.
         */
        int tailOffset;

        /**
         * If the observers are {@link #TERMINATED}, this holds the terminal error if not null.
         */
        Throwable error;

        /**
         * True if the source has terminated.
         */
        volatile boolean done;

        @SuppressWarnings("unchecked")
        Multicaster(int capacityHint, final Node<T> head) {
            super(EMPTY);
            this.tail = head;
Repository: RustPython/RustPython
File: extra_tests/snippets/syntax_annotations.py (Python)

Code before:
from typing import get_type_hints

def func(s: str) -> int:
    return int(s)

hints = get_type_hints(func)

# The order of type hints matters for certain functions
# e.g. functools.singledispatch
assert hints == {'s': str, 'return': int}


Review comment (suggestion):
This test doesn't check the order. It have to be serialized to check the order.

```suggestion
assert list(hints.items()) == [('s', str), ('return': int)]
```

Code after:
from typing import get_type_hints

def func(s: str) -> int:
    return int(s)

hints = get_type_hints(func)

# The order of type hints matters for certain functions
# e.g. functools.singledispatch
assert list(hints.items()) == [('s', str), ('return', int)]
Repository: RustPython/RustPython
File: crates/vm/src/stdlib/ctypes/base.rs (Rust)

Code before:

#[pyclass(flags(BASETYPE))]
impl PyCSimpleType {
    #[allow(clippy::new_ret_no_self)]
    #[pymethod]
    fn new(cls: PyTypeRef, _: OptionalArg, vm: &VirtualMachine) -> PyResult {
        Ok(PyObjectRef::from(
            new_simple_type(Either::B(&cls), vm)?
                .into_ref_with_type(vm, cls)?
                .clone(),
        ))
    }

    #[pyclassmethod]
    fn from_param(cls: PyTypeRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult {
        // If the value is already an instance of the requested type, return it
        if value.fast_isinstance(&cls) {
            return Ok(value);
        }

        // Check for _as_parameter_ attribute
        if let Ok(as_parameter) = value.get_attr("_as_parameter_", vm) {
            return PyCSimpleType::from_param(cls, as_parameter, vm);
        }

        Err(vm.new_type_error("wrong type".to_string()))
    }
}

#[pyclass(
    module = "_ctypes",
    name = "_SimpleCData",
    base = PyCData,
    metaclass = "PyCSimpleType"
)]
#[derive(PyPayload)]
pub struct PyCSimple {
    pub _type_: String,
    pub value: AtomicCell<PyObjectRef>,
}

impl Debug for PyCSimple {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.debug_struct("PyCSimple")
            .field("_type_", &self._type_)
            .finish()
    }
}

impl Constructor for PyCSimple {
    type Args = (OptionalArg,);

Review comment (bug):
```suggestion
        Err(vm.new_type_error("wrong type"))
```

Code after:

#[pyclass(flags(BASETYPE))]
impl PyCSimpleType {
    #[allow(clippy::new_ret_no_self)]
    #[pymethod]
    fn new(cls: PyTypeRef, _: OptionalArg, vm: &VirtualMachine) -> PyResult {
        Ok(PyObjectRef::from(
            new_simple_type(Either::B(&cls), vm)?
                .into_ref_with_type(vm, cls)?
                .clone(),
        ))
    }

    #[pyclassmethod]
    fn from_param(cls: PyTypeRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult {
        // If the value is already an instance of the requested type, return it
        if value.fast_isinstance(&cls) {
            return Ok(value);
        }

        // Check for _as_parameter_ attribute
        let Ok(as_parameter) = value.get_attr("_as_parameter_", vm) else {
            return Err(vm.new_type_error("wrong type"));
        };

        PyCSimpleType::from_param(cls, as_parameter, vm)
    }
}

#[pyclass(
    module = "_ctypes",
    name = "_SimpleCData",
    base = PyCData,
    metaclass = "PyCSimpleType"
)]
#[derive(PyPayload)]
pub struct PyCSimple {
    pub _type_: String,
    pub value: AtomicCell<PyObjectRef>,
}

impl Debug for PyCSimple {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.debug_struct("PyCSimple")
            .field("_type_", &self._type_)
            .finish()
    }
}

impl Constructor for PyCSimple {
    type Args = (OptionalArg,);
Repository: RustPython/RustPython
File: crates/compiler-core/src/bytecode.rs (Rust)

Code before:
        /// ```py
        /// foo is bar
        /// x in lst
        /// ```
        No = 0,
        /// ```py
        /// foo is not bar
        /// x not in lst
        /// ```
        Yes = 1,
    }
);

op_arg_enum!(
    /// Specifies if a slice is built with either 2 or 3 arguments.
    #[repr(u8)]
    #[derive(Clone, Copy, Debug, Eq, PartialEq)]
    pub enum BuildSliceArgCount {
        /// ```py
        /// x[5:10]
        /// ```
        Two = 2,
        /// ```py
        /// x[5:10:2]
        /// ```
        Three = 3,
    }
);

#[derive(Copy, Clone)]
pub struct UnpackExArgs {
    pub before: u8,
    pub after: u8,
}

impl OpArgType for UnpackExArgs {
    #[inline(always)]
    fn from_op_arg(x: u32) -> Option<Self> {
        let [before, after, ..] = x.to_le_bytes();
        Some(Self { before, after })
    }

    #[inline(always)]
    fn to_op_arg(self) -> u32 {
        u32::from_le_bytes([self.before, self.after, 0, 0])
    }
}

impl fmt::Display for UnpackExArgs {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "before: {}, after: {}", self.before, self.after)

Review comment (suggestion):
```suggestion
        /// ```py
        /// x[5:10]
        /// ```
        Two,
        /// ```py
        /// x[5:10:2]
        /// ```
        Three,
```
Removing value not to block optimization. Adding a method will be enough

```rust
impl BuildSliceArgCount {
    const fn argc(&self) -> usize {
        ....
    }
}

Code after:
        /// ```py
        /// foo is bar
        /// x in lst
        /// ```
        No = 0,
        /// ```py
        /// foo is not bar
        /// x not in lst
        /// ```
        Yes = 1,
    }
);

/// Specifies if a slice is built with either 2 or 3 arguments.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum BuildSliceArgCount {
    /// ```py
    /// x[5:10]
    /// ```
    Two,
    /// ```py
    /// x[5:10:2]
    /// ```
    Three,
}

impl OpArgType for BuildSliceArgCount {
    #[inline(always)]
    fn from_op_arg(x: u32) -> Option<Self> {
        Some(match x {
            2 => Self::Two,
            3 => Self::Three,
            _ => return None,
        })
    }

    #[inline(always)]
    fn to_op_arg(self) -> u32 {
        u32::from(self.argc().get())
    }
}

impl BuildSliceArgCount {
    /// Get the numeric value of `Self`.
    #[must_use]
    pub const fn argc(self) -> NonZeroU8 {
        let inner = match self {
            Self::Two => 2,
            Self::Three => 3,
        };
        // Safety: `inner` can be either 2 or 3.
Repository: RustPython/RustPython
File: crates/compiler-core/src/bytecode.rs (Rust)

Code before:
    BuildTupleFromIter = 123,
    BuildTupleFromTuples {
        size: Arg<u32>,
    } = 124,
    CallMethodPositional {
        nargs: Arg<u32>,
    } = 125,
    CallMethodKeyword {
        nargs: Arg<u32>,
    } = 126,
    CallMethodEx {
        has_kwargs: Arg<bool>,
    } = 127,
    Continue {
        target: Arg<Label>,
    } = 128,
    JumpIfFalseOrPop {
        target: Arg<Label>,
    } = 129,
    JumpIfTrueOrPop {
        target: Arg<Label>,
    } = 130,
    JumpIfNotExcMatch(Arg<Label>) = 131,
    LoadClassDeref(Arg<NameIdx>) = 132,
    SetExcInfo = 133,
    Subscript = 134,
    // ===== Pseudo Opcodes (252+) ======
    Jump {
        target: Arg<Label>,
    } = 252, // CPython uses pseudo-op 256
    LoadClosure(Arg<NameIdx>) = 253, // CPython uses pseudo-op 258
    LoadMethod {
        idx: Arg<NameIdx>,
    } = 254, // CPython uses pseudo-op 259
    PopBlock = 255,                  // CPython uses pseudo-op 263
}

const _: () = assert!(mem::size_of::<Instruction>() == 1);

impl From<Instruction> for u8 {
    #[inline]
    fn from(ins: Instruction) -> Self {
        // SAFETY: there's no padding bits
        unsafe { core::mem::transmute::<Instruction, Self>(ins) }
    }
}

impl TryFrom<u8> for Instruction {
    type Error = MarshalError;

    #[inline]

Review comment (suggestion):
```suggestion
    SetExcInfo = 134,
    Subscript = 135,
```

Code after:
    BuildTupleFromIter = 123,
    BuildTupleFromTuples {
        size: Arg<u32>,
    } = 124,
    Continue {
        target: Arg<Label>,
    } = 128,
    JumpIfFalseOrPop {
        target: Arg<Label>,
    } = 129,
    JumpIfTrueOrPop {
        target: Arg<Label>,
    } = 130,
    JumpIfNotExcMatch(Arg<Label>) = 131,
    LoadClassDeref(Arg<NameIdx>) = 132,
    SetExcInfo = 134,
    Subscript = 135,
    // ===== Pseudo Opcodes (252+) ======
    Jump {
        target: Arg<Label>,
    } = 252, // CPython uses pseudo-op 256
    LoadClosure(Arg<NameIdx>) = 253, // CPython uses pseudo-op 258
    LoadAttrMethod {
        idx: Arg<NameIdx>,
    } = 254, // CPython uses pseudo-op 259
    PopBlock = 255,                  // CPython uses pseudo-op 263
}

const _: () = assert!(mem::size_of::<Instruction>() == 1);

impl From<Instruction> for u8 {
    #[inline]
    fn from(ins: Instruction) -> Self {
        // SAFETY: there's no padding bits
        unsafe { core::mem::transmute::<Instruction, Self>(ins) }
    }
}

impl TryFrom<u8> for Instruction {
    type Error = MarshalError;

    #[inline]
    fn try_from(value: u8) -> Result<Self, MarshalError> {
        // CPython-compatible opcodes (0-118)
        let cpython_start = u8::from(Self::Cache);
        let cpython_end = u8::from(Self::YieldValue { arg: Arg::marker() });

        // Resume has a non-contiguous opcode (149)
        let resume_id = u8::from(Self::Resume { arg: Arg::marker() });

        // RustPython-only opcodes (explicit list to avoid gaps like 125-127)
Repository: RustPython/RustPython
File: crates/stdlib/src/pyexpat.rs (Rust)

Code before:
        args: ParserCreateArgs,
        vm: &VirtualMachine,
    ) -> PyResult<PyExpatLikeXmlParserRef> {
        // Validate namespace_separator: must be at most one character
        let ns_sep = match args.namespace_separator {
            Some(ref s) => {
                let chars: Vec<char> = s.as_str().chars().collect();
                if chars.len() > 1 {
                    return Err(vm.new_value_error(
                        "namespace_separator must be at most one character, omitted, or None"
                            .to_owned(),
                    ));
                }
                Some(s.as_str().to_owned())
            }
            None => None,
        };

        // encoding parameter is currently not used (xml-rs handles encoding from XML declaration)
        let _ = args.encoding;

        PyExpatLikeXmlParser::new(ns_sep, args.intern, vm)
    }

    // TODO: Tie this exception to the module's state.
    // TODO: Alias `error` to `ExpatError`
    #[pyattr]
    #[pyexception(name = "ExpatError", base = PyException)]
    #[derive(Debug)]
    #[repr(transparent)]
    pub struct PyExpatError(PyException);

    #[pyexception]
    impl PyExpatError {}
}

#[pymodule(name = "model")]
mod _model {}

#[pymodule(name = "errors")]
mod _errors {}


Review comment (suggestion):
```suggestion
    #[pyattr]
    #[pyattr(name = "error")]
```
This will add alias error

Code after:
        args: ParserCreateArgs,
        vm: &VirtualMachine,
    ) -> PyResult<PyExpatLikeXmlParserRef> {
        // Validate namespace_separator: must be at most one character
        let ns_sep = match args.namespace_separator {
            Some(ref s) => {
                let chars: Vec<char> = s.as_str().chars().collect();
                if chars.len() > 1 {
                    return Err(vm.new_value_error(
                        "namespace_separator must be at most one character, omitted, or None"
                            .to_owned(),
                    ));
                }
                Some(s.as_str().to_owned())
            }
            None => None,
        };

        // encoding parameter is currently not used (xml-rs handles encoding from XML declaration)
        let _ = args.encoding;

        PyExpatLikeXmlParser::new(ns_sep, args.intern, vm)
    }

    // TODO: Tie this exception to the module's state.
    #[pyattr]
    #[pyattr(name = "error")]
    #[pyexception(name = "ExpatError", base = PyException)]
    #[derive(Debug)]
    #[repr(transparent)]
    pub struct PyExpatError(PyException);

    #[pyexception]
    impl PyExpatError {}
}

#[pymodule(name = "model")]
mod _model {}

#[pymodule(name = "errors")]
mod _errors {}
Repository: RustPython/RustPython
File: extra_tests/snippets/syntax_short_circuit_bool.py (Python)

Code before:
class ExplodingBool():
    def __init__(self, value):
        self.value = value
        self.booled = False

    def __bool__(self):
        assert not self.booled
        self.booled = True
        return self.value

y = (ExplodingBool(False) and False and True and False)
print(y)

if (ExplodingBool(True) or False or True or False):
    pass

assert ExplodingBool(True) or False

while ExplodingBool(False) and False:
    pass

# if ExplodingBool(False) and False and True and False:
#     pass

# Issue #3567: nested BoolOps should not call __bool__ redundantly
ExplodingBool(False) and False or False


Review comment (suggestion):
```suggestion
assert ExplodingBool(False) and False or False
```

Code after:
class ExplodingBool():
    def __init__(self, value):
        self.value = value
        self.booled = False

    def __bool__(self):
        assert not self.booled
        self.booled = True
        return self.value

y = (ExplodingBool(False) and False and True and False)
print(y)

if (ExplodingBool(True) or False or True or False):
    pass

assert ExplodingBool(True) or False

while ExplodingBool(False) and False:
    pass

# if ExplodingBool(False) and False and True and False:
#     pass

# Issue #3567: nested BoolOps should not call __bool__ redundantly
assert (ExplodingBool(False) and False or False) == False