ronantakizawa/codereview-bench huggingface dataset:
Repository: dotnet/runtime
File: src/native/corehost/browserhost/host/cross-linked.ts (TypeScript)

Code before:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

import { } from "../../../libs/Common/JavaScript/cross-linked";
import { VoidPtr } from "./types";

declare global {
    export const BROWSER_HOST: any;
    export function _BrowserHost_ExecuteAssembly(mainAssemblyNamePtr: number, argsLength: number, argsPtr: number): number;
    export function _wasm_load_icu_data(dataPtr: VoidPtr): number;
}


Review comment (suggestion):
```diff
-import { VoidPtr } from "./types";
+import type { VoidPtr } from "./types";
```

Code after:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

import { } from "../../../libs/Common/JavaScript/cross-linked";
import type { VoidPtr } from "./types";

declare global {
    export const BROWSER_HOST: any;
    export function _BrowserHost_ExecuteAssembly(mainAssemblyNamePtr: number, argsLength: number, argsPtr: number): number;
    export function _wasm_load_icu_data(dataPtr: VoidPtr): number;
}
Repository: dotnet/runtime
File: src/coreclr/jit/objectalloc.cpp (C++)

Code before:
        JITDUMPEXEC(DumpIndex(c->m_pseudoIndex));
        JITDUMP(" unable to clone\n");

        overlaps = true;
        break;
    }

    return overlaps;
}

//------------------------------------------------------------------------------
// ShouldClone: check if this cloning looks profitable
//
// Arguments:
//   info        -- info about a cloning opportunity
//
// Returns:
//   true if cloning looks profitable
//
bool ObjectAllocator::ShouldClone(CloneInfo* info)
{
    // For now, use the same cloning size limit we use for loop cloning
    //
    int const      sizeConfig  = JitConfig.JitCloneLoopsSizeLimit();
    unsigned const sizeLimit   = (sizeConfig >= 0) ? (unsigned)sizeConfig : UINT_MAX;
    unsigned       size        = 0;
    bool           shouldClone = true;
    auto           countNode   = [&size](GenTree* tree) -> unsigned {
        size++;
        return 1;
    };

    for (BasicBlock* const block : *info->m_blocksToClone)
    {
        // Note this overstates the size a bit since we'll resolve GDVs
        // in the clone and the original...
        //
        unsigned const slack = sizeLimit - size;
        if (block->ComplexityExceeds(comp, slack, countNode))
        {
            JITDUMP("Rejecting");
            JITDUMPEXEC(DumpIndex(info->m_pseudoIndex));
            JITDUMP(" cloning: exceeds size limit %u\n", sizeLimit);
            return false;
        }
    }

    // TODO: some kind of profile check...
    //
    JITDUMP("Accepting");
    JITDUMPEXEC(DumpIndex(info->m_pseudoIndex));

Review comment (bug):
Incorrect article: should be "a conditional" instead of "an conditional".
```suggestion
//    part of a conditional escape optimization
```

Code after:
        if (!c->m_willClone)
        {
            continue;
        }

        if (BitVecOps::IsEmptyIntersection(&traits, info->m_blocks, c->m_blocks))
        {
            continue;
        }

        JITDUMP("Cloned blocks for");
        JITDUMPEXEC(DumpIndex(info->m_pseudoIndex));
        JITDUMP(" overlap with those for");
        JITDUMPEXEC(DumpIndex(c->m_pseudoIndex));
        JITDUMP(" unable to clone\n");

        overlaps = true;
        break;
    }

    return overlaps;
}

//------------------------------------------------------------------------------
// BlockIsCloneOrWasCloned: check if this block is a clone or was cloned as
//    part of an conditional escape optimization
//
// Arguments:
//   block -- block in question
//
// Returns:
//   true if block was cloned
//
// Notes:
//   Such blocks are interesting when they contain ALLCOBJs that we can prove
//   won't escape.
//
bool ObjectAllocator::BlockIsCloneOrWasCloned(BasicBlock* block)
{
    if (block->bbID >= m_initialMaxBlockID)
    {
        JITDUMP("Block " FMT_BB " was cloned as part of conditional escape processing\n", block->bbNum);
        return true;
    }

    BitVecTraits traits(m_initialMaxBlockID, comp);

    for (CloneInfo* const c : CloneMap::ValueIteration(&m_CloneMap))
    {
        // Ignore regions that we did not clone
        //
Repository: dotnet/runtime
File: src/coreclr/jit/rangecheck.h (C/C++)

Code before:
        // Check hi ranges if they are dependent and not unknown.
        if ((r1hi.IsDependent() && !r1hi.IsUnknown()) || (r2hi.IsDependent() && !r2hi.IsUnknown()))
        {
            result.uLimit = Limit(Limit::keDependent);
        }

        if (r1lo.IsConstant())
        {
            result.lLimit = AddConstantLimit(r2lo, r1lo);
        }
        if (r2lo.IsConstant())
        {
            result.lLimit = AddConstantLimit(r1lo, r2lo);
        }
        if (r1hi.IsConstant())
        {
            result.uLimit = AddConstantLimit(r2hi, r1hi);
        }
        if (r2hi.IsConstant())
        {
            result.uLimit = AddConstantLimit(r1hi, r2hi);
        }
        return result;
    }

    // Given two ranges "r1" and "r2", perform an add operation on the
    // ranges.
    static Range Subtract(Range& r1, Range& r2)
    {
        Limit& r1lo = r1.LowerLimit();
        Limit& r1hi = r1.UpperLimit();
        Limit& r2lo = r2.LowerLimit();
        Limit& r2hi = r2.UpperLimit();

        Range result = Limit(Limit::keUnknown);

        // Check lo ranges if they are dependent and not unknown.
        if ((r1lo.IsDependent() && !r1lo.IsUnknown()) || (r2lo.IsDependent() && !r2lo.IsUnknown()))
        {
            result.lLimit = Limit(Limit::keDependent);
        }
        // Check hi ranges if they are dependent and not unknown.
        if ((r1hi.IsDependent() && !r1hi.IsUnknown()) || (r2hi.IsDependent() && !r2hi.IsUnknown()))
        {
            result.uLimit = Limit(Limit::keDependent);
        }

        if (r1lo.IsConstant())
        {
            result.lLimit = SubtractConstantLimit(r2lo, r1lo);
        }

Review comment (suggestion):
```suggestion
    // Given two ranges "r1" and "r2", perform a subtract operation on the
```

Code after:
        }
        // Check hi ranges if they are dependent and not unknown.
        if ((r1hi.IsDependent() && !r1hi.IsUnknown()) || (r2hi.IsDependent() && !r2hi.IsUnknown()))
        {
            result.uLimit = Limit(Limit::keDependent);
        }

        if (r1lo.IsConstant())
        {
            result.lLimit = op(r2lo, r1lo);
        }
        if (r2lo.IsConstant())
        {
            result.lLimit = op(r1lo, r2lo);
        }
        if (r1hi.IsConstant())
        {
            result.uLimit = op(r2hi, r1hi);
        }
        if (r2hi.IsConstant())
        {
            result.uLimit = op(r1hi, r2hi);
        }

        return result;
    }

    static Range Add(Range& r1, Range& r2)
    {
        return ApplyRangeOp(r1, r2, [](Limit& a, Limit& b) {
            return AddConstantLimit(a, b);
        });
    }

    static Range Subtract(Range& r1, Range& r2)
    {
        return ApplyRangeOp(r1, r2, [](Limit& a, Limit& b) {
            return SubtractConstantLimit(a, b);
        });
    }

    static Range Multiply(Range& r1, Range& r2)
    {
        return ApplyRangeOp(r1, r2, [](Limit& a, Limit& b) {
            return MultiplyConstantLimit(a, b);
        });
    }

    static Range ShiftRight(Range& r1, Range& r2)
    {
        Limit& r1lo = r1.LowerLimit();
Repository: dotnet/runtime
File: src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCompressionMethod.cs (C#)

Code before:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace System.IO.Compression
{
    /// <summary>
    /// Specifies the compression method used to compress an entry in a zip archive.
    /// </summary>
    /// <remarks>
    /// The values correspond to the compression method values described in the ZIP File Format Specification (APPNOTE.TXT section 4.4.5).
    /// </remarks>
    public enum ZipCompressionMethod : short
    {
        /// <summary>
        /// The entry is stored (no compression).
        /// </summary>
        Stored = 0x0,

        /// <summary>
        /// The entry is compressed using the Deflate algorithm.
        /// </summary>
        Deflate = 0x8,

        /// <summary>
        /// The entry is compressed using the Deflate64 algorithm.
        /// </summary>
        Deflate64 = 0x9,
    }
}


Review comment (suggestion):
The integer is unsigned in the approved proposal.

```suggestion
    public enum ZipCompressionMethod : ushort
```

Code after:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace System.IO.Compression
{
    /// <summary>
    /// Specifies the compression method used to compress an entry in a zip archive.
    /// </summary>
    /// <remarks>
    /// The values correspond to the compression method values described in the ZIP File Format Specification (APPNOTE.TXT section 4.4.5).
    /// </remarks>
    public enum ZipCompressionMethod
    {
        /// <summary>
        /// The entry is stored (no compression).
        /// </summary>
        Stored = 0x0,

        /// <summary>
        /// The entry is compressed using the Deflate algorithm.
        /// </summary>
        Deflate = 0x8,

        /// <summary>
        /// The entry is compressed using the Deflate64 algorithm.
        /// </summary>
        Deflate64 = 0x9,
    }
}
Repository: dotnet/runtime
File: src/coreclr/tools/Common/Compiler/ObjectWriter/WasmNative.cs (C#)

Code before:
using System;
using System.Buffers;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Numerics;
using System.Runtime.CompilerServices;

namespace ILCompiler.ObjectWriter
{
    public enum WasmSectionType
    {
        Custom = 0,
        Type = 1,
        Import = 2,
        Function = 3,
        Table = 4,
        Memory = 5,
        Global = 6,
        Export = 7,
        Start = 8,
        Element = 9,
        Code = 10,
        Data = 11,
        DataCount = 12,
        Tag = 13,
    }


Review comment (suggestion):
nit: missing header

```suggestion
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
```

Code after:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using System.Linq;
using System.Collections.Generic;

namespace ILCompiler.ObjectWriter
{
    public enum WasmSectionType
    {
        Custom = 0,
        Type = 1,
        Import = 2,
        Function = 3,
        Table = 4,
        Memory = 5,
        Global = 6,
        Export = 7,
        Start = 8,
        Element = 9,
        Code = 10,
        Data = 11,
        DataCount = 12,
        Tag = 13,
    }

    public static class PlaceholderValues
Repository: dotnet/runtime
File: src/coreclr/tools/Common/Compiler/ObjectWriter/WasmObjectWriter.cs (C#)

Code before:
﻿using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Numerics;
using System.Runtime.InteropServices;
using System.Runtime.Serialization;
using System.Text;
using System.Xml.Linq;
using ILCompiler.DependencyAnalysis;
using ILCompiler.DependencyAnalysisFramework;
using Internal.TypeSystem;

namespace ILCompiler.ObjectWriter
{
    // This is a placeholder for now. It will need to be filled in with ABI-specific signatures
    // for any method we want to emit into the Wasm module.
    public class WasmAbiContext
    {
        private Dictionary<MethodDesc, WasmFuncType> _methodSignatureMap = new();
        public WasmFuncType GetSignature(MethodDesc method)
        {
            return DummyValues.CreateWasmFunc_i32_i32();
        }
    }


Review comment (suggestion):
```suggestion
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
```

Code after:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Buffers;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Text;
using ILCompiler.DependencyAnalysis;
using ILCompiler.DependencyAnalysis.ReadyToRun;
using ILCompiler.DependencyAnalysisFramework;
using Internal.Text;
using Internal.TypeSystem;

using ObjectData = ILCompiler.DependencyAnalysis.ObjectNode.ObjectData;

namespace ILCompiler.ObjectWriter
{
    /// <summary>
    /// Wasm object file format writer.
    /// </summary>
    internal sealed class WasmObjectWriter : ObjectWriter
    {
        public WasmObjectWriter(NodeFactory factory, ObjectWritingOptions options, OutputInfoBuilder outputInfoBuilder)
Repository: dotnet/runtime
File: src/coreclr/tools/Common/Compiler/ObjectWriter/WasmObjectWriter.cs (C#)

Code before:
﻿using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Numerics;
using System.Runtime.InteropServices;
using System.Runtime.Serialization;
using System.Text;
using System.Xml.Linq;
using ILCompiler.DependencyAnalysis;
using ILCompiler.DependencyAnalysisFramework;
using Internal.TypeSystem;

namespace ILCompiler.ObjectWriter
{
    // This is a placeholder for now. It will need to be filled in with ABI-specific signatures
    // for any method we want to emit into the Wasm module.
    public class WasmAbiContext
    {
        private Dictionary<MethodDesc, WasmFuncType> _methodSignatureMap = new();
        public WasmFuncType GetSignature(MethodDesc method)
        {
            return DummyValues.CreateWasmFunc_i32_i32();
        }
    }

    internal class WasmSection
    {
        public WasmSectionType Type { get; }
        public string Name { get; }
        public SectionData Data => _data;
        private SectionWriter? _writer;

        private SectionData _data;

Review comment (suggestion):
Several unused using directives are present: `System.Numerics`, `System.Runtime.InteropServices`, `System.Runtime.Serialization`, and `System.Xml.Linq`. These should be removed to keep the code clean.
```suggestion
using System.Text;
```

Code after:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Buffers;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Text;
using ILCompiler.DependencyAnalysis;
using ILCompiler.DependencyAnalysis.ReadyToRun;
using ILCompiler.DependencyAnalysisFramework;
using Internal.Text;
using Internal.TypeSystem;

using ObjectData = ILCompiler.DependencyAnalysis.ObjectNode.ObjectData;

namespace ILCompiler.ObjectWriter
{
    /// <summary>
    /// Wasm object file format writer.
    /// </summary>
    internal sealed class WasmObjectWriter : ObjectWriter
    {
        public WasmObjectWriter(NodeFactory factory, ObjectWritingOptions options, OutputInfoBuilder outputInfoBuilder)
            : base(factory, options, outputInfoBuilder)
        {
        }

        private void EmitWasmHeader(Stream outputFileStream)
        {
            outputFileStream.Write("\0asm"u8);
            outputFileStream.Write([0x1, 0x0, 0x0, 0x0]);
Repository: dotnet/runtime
File: src/coreclr/tools/Common/Compiler/ObjectWriter/WasmNative.cs (C#)

Code before:
using System;
using System.Buffers;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Numerics;
using System.Runtime.CompilerServices;

namespace ILCompiler.ObjectWriter
{
    public enum WasmSectionType
    {
        Custom = 0,
        Type = 1,
        Import = 2,
        Function = 3,
        Table = 4,
        Memory = 5,
        Global = 6,
        Export = 7,
        Start = 8,
        Element = 9,
        Code = 10,
        Data = 11,
        DataCount = 12,
        Tag = 13,
    }

    public static class DummyValues
    {
        // Wasm function signature for (func (params i32) (result i32))
        public static WasmFuncType CreateWasmFunc_i32_i32()

Review comment (suggestion):
Several unused using directives are present: `System.Buffers`, `System.Diagnostics.CodeAnalysis`, `System.IO`, `System.Numerics`, and `System.Runtime.CompilerServices`. These should be removed to keep the code clean.
```suggestion
using System.Diagnostics;
using System.Linq;
```

Code after:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using System.Linq;
using System.Collections.Generic;

namespace ILCompiler.ObjectWriter
{
    public enum WasmSectionType
    {
        Custom = 0,
        Type = 1,
        Import = 2,
        Function = 3,
        Table = 4,
        Memory = 5,
        Global = 6,
        Export = 7,
        Start = 8,
        Element = 9,
        Code = 10,
        Data = 11,
        DataCount = 12,
        Tag = 13,
    }

    public static class PlaceholderValues
    {
        // Wasm function signature for (func (params i32) (result i32))
        public static WasmFuncType CreateWasmFunc_i32_i32()
        {
Repository: dotnet/runtime
File: src/coreclr/jit/codegenwasm.cpp (C++)

Code before:
    }
    else
    {
        assert(genIsValidReg(varDsc->GetRegNum()));
        unsigned wasmLclIndex = UnpackWasmReg(varDsc->GetRegNum());
        GetEmitter()->emitIns_I(INS_local_get, emitTypeSize(tree), wasmLclIndex);
    }
}

BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
    assert(block->KindIs(BBJ_CALLFINALLY));
    NYI_WASM("genCallFinally");
    return nullptr;
}

void CodeGen::genEHCatchRet(BasicBlock* block)
{
    NYI_WASM("genEHCatchRet");
}

void CodeGen::genStructReturn(GenTree* treeNode)
{
    NYI_WASM("genStructReturn");
}

void CodeGen::genEmitGSCookieCheck(bool tailCall)
{
    // TODO-WASM: GS cookie checks have limited utility on WASM since they can only help
    // with detecting linear memory stack corruption. Decide if we want them anyway.
    NYI_WASM("genEmitGSCookieCheck");
}

void CodeGen::genProfilingLeaveCallback(unsigned helper)
{
    NYI_WASM("genProfilingLeaveCallback");
}

void CodeGen::genSpillVar(GenTree* tree)
{
    NYI_WASM("Put all spillng to memory under '#if HAS_FIXED_REGISTER_SET'");
}

//------------------------------------------------------------------------
// inst_JMP: Generate a jump instruction.
//
void CodeGen::inst_JMP(emitJumpKind jmp, BasicBlock* tgtBlock)
{
    NYI_WASM("inst_JMP");
}


Review comment (suggestion):
Per our discussion, this should also be moved out (`genCodeForDivMod`).

Code after:
            break;
        case PackOperAndType(GT_SUB, TYP_FLOAT):
            ins = INS_f32_sub;
            break;
        case PackOperAndType(GT_SUB, TYP_DOUBLE):
            ins = INS_f64_sub;
            break;

        case PackOperAndType(GT_MUL, TYP_INT):
            if (treeNode->gtOverflow())
                NYI_WASM("Overflow checks");
            ins = INS_i32_mul;
            break;
        case PackOperAndType(GT_MUL, TYP_LONG):
            if (treeNode->gtOverflow())
                NYI_WASM("Overflow checks");
            ins = INS_i64_mul;
            break;
        case PackOperAndType(GT_MUL, TYP_FLOAT):
            ins = INS_f32_mul;
            break;
        case PackOperAndType(GT_MUL, TYP_DOUBLE):
            ins = INS_f64_mul;
            break;

        case PackOperAndType(GT_DIV, TYP_INT):
            ins = INS_i32_div_s;
            break;
        case PackOperAndType(GT_DIV, TYP_LONG):
            ins = INS_i64_div_s;
            break;
        case PackOperAndType(GT_DIV, TYP_FLOAT):
            ins = INS_f32_div;
            break;
        case PackOperAndType(GT_DIV, TYP_DOUBLE):
            ins = INS_f64_div;
            break;

        case PackOperAndType(GT_UDIV, TYP_INT):
            ins = INS_i32_div_u;
            break;
        case PackOperAndType(GT_UDIV, TYP_LONG):
            ins = INS_i64_div_u;
            break;

        case PackOperAndType(GT_MOD, TYP_INT):
            ins = INS_i32_rem_s;
            break;
        case PackOperAndType(GT_MOD, TYP_LONG):
            ins = INS_i64_rem_s;
            break;
Repository: dotnet/runtime
File: src/tests/JIT/Regression/JitBlue/Runtime_120903/Runtime_120903.cs (C#)

Code before:

namespace Runtime_120903;

using System;
using System.Runtime.CompilerServices;
using Xunit;

class B {}
class D1: B {}
class D2: B {}

public class Runtime_120903
{
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    static int Merge(object[] b)
    {
        D1[] d1 = Unsafe.As<D1[]>(b);
        return d1.Length;
    }

    [Fact]
    public static void Problem()
    {
        D2[] d2 = new D2[10];
        Console.WriteLine(Merge(d2));
    }   
}


Review comment (suggestion):
This line contains trailing whitespace. According to the coding guidelines, markdown files should have no trailing whitespace at the end of any line. While this is a .cs file, the same principle applies for consistency and clean diffs.
```suggestion
    }
```

Code after:

namespace Runtime_120903;

using System;
using System.Runtime.CompilerServices;
using Xunit;

class B {}
class D1: B {}
class D2: B {}

public class Runtime_120903
{
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    static int Merge(object[] b)
    {
        D1[] d1 = Unsafe.As<D1[]>(b);
        return d1.Length;
    }

    [Fact]
    public static void Problem()
    {
        D2[] d2 = new D2[10];
        Console.WriteLine(Merge(d2));
    }
}