ronantakizawa/codereview-bench huggingface dataset:
Repository: dolthub/dolt
File: go/store/blobstore/git_blobstore_chunked_put_test.go (Go)

Code before:

	// blob stays blob (even if the caller would have triggered chunked mode).
	// Non-manifest Put is deferred; idempotent re-Put should return same version from cache.
	verBlob, err := bs.Put(ctx, "k", 2, bytes.NewReader([]byte("hi")))
	require.NoError(t, err)
	verNoop, err := bs.Put(ctx, "k", 10, putShouldNotRead{})
	require.NoError(t, err)
	require.Equal(t, verBlob, verNoop)

	got, _, err := GetBytes(ctx, bs, "k", AllRange)
	require.NoError(t, err)
	require.Equal(t, []byte("hi"), got)

	// tree stays tree
	verTree, err := bs.Put(ctx, "ktree", 10, bytes.NewReader([]byte("abcdefghij")))
	require.NoError(t, err)
	verTreeNoop, err := bs.Put(ctx, "ktree", 2, putShouldNotRead{})
	require.NoError(t, err)
	require.Equal(t, verTree, verTreeNoop)

	got, _, err = GetBytes(ctx, bs, "ktree", AllRange)
	require.NoError(t, err)
	require.Equal(t, []byte("abcdefghij"), got)

	// Flush deferred writes and verify remote state.
	_, err = bs.CheckAndPut(ctx, "", "manifest", 3, bytes.NewReader([]byte("m1\n")))
	require.NoError(t, err)

	head, ok, err := api.TryResolveRefCommit(ctx, DoltDataRef)
	require.NoError(t, err)
	require.True(t, ok)

	_, typ, err := api.ResolvePathObject(ctx, head, "k")
	require.NoError(t, err)
	require.Equal(t, git.ObjectTypeBlob, typ)

	_, typ, err = api.ResolvePathObject(ctx, head, "ktree")
	require.NoError(t, err)
	require.Equal(t, git.ObjectTypeTree, typ)
}


Review comment (suggestion):
nit. the use of `m1\n` makes is look like this is a magic string, when I think it's just like saying `foobar\n` - so it's clear to people that it's nonsense data input.

Code after:

	// blob stays blob (even if the caller would have triggered chunked mode).
	// Non-manifest Put is deferred; idempotent re-Put should return same version from cache.
	verBlob, err := bs.Put(ctx, "k", 2, bytes.NewReader([]byte("hi")))
	require.NoError(t, err)
	verNoop, err := bs.Put(ctx, "k", 10, putShouldNotRead{})
	require.NoError(t, err)
	require.Equal(t, verBlob, verNoop)

	got, _, err := GetBytes(ctx, bs, "k", AllRange)
	require.NoError(t, err)
	require.Equal(t, []byte("hi"), got)

	// tree stays tree
	verTree, err := bs.Put(ctx, "ktree", 10, bytes.NewReader([]byte("abcdefghij")))
	require.NoError(t, err)
	verTreeNoop, err := bs.Put(ctx, "ktree", 2, putShouldNotRead{})
	require.NoError(t, err)
	require.Equal(t, verTree, verTreeNoop)

	got, _, err = GetBytes(ctx, bs, "ktree", AllRange)
	require.NoError(t, err)
	require.Equal(t, []byte("abcdefghij"), got)

	// Flush deferred writes and verify remote state.
	_, err = bs.CheckAndPut(ctx, "", "manifest", 3, bytes.NewReader([]byte("xxx\n")))
	require.NoError(t, err)

	head, ok, err := api.TryResolveRefCommit(ctx, DoltDataRef)
	require.NoError(t, err)
	require.True(t, ok)

	_, typ, err := api.ResolvePathObject(ctx, head, "k")
	require.NoError(t, err)
	require.Equal(t, git.ObjectTypeBlob, typ)

	_, typ, err = api.ResolvePathObject(ctx, head, "ktree")
	require.NoError(t, err)
	require.Equal(t, git.ObjectTypeTree, typ)
}
Repository: dolthub/dolt
File: .github/scripts/performance-benchmarking/get-mysql-dolt-job-json.sh (Shell)

Code before:
#!/bin/sh

set -e

if [ "$#" -lt 12 ]; then
    echo  "Usage: ./get-job-json.sh <jobname> <fromServer> <fromVersion> <toServer> <toVersion> <timeprefix> <actorprefix> <format> <issueNumber> <initBigRepo> <nomsBinFormat> [sysbenchTestTime] <withTpcc>"
    exit 1
fi

jobname="$1"
fromServer="$2"
fromVersion="$3"
toServer="$4"
toVersion="$5"
timeprefix="$6"
actorprefix="$7"
format="$8"
issueNumber="$9"
initBigRepo="${10}"
nomsBinFormat="${11}"
sysbenchTestTime=""
withTpcc=""
if [ "$#" -ge 13 ]; then
  sysbenchTestTime="${12}"
  withTpcc="${13}"
else
  withTpcc="${12}"
fi
precision="2"
tpccRegex="tpcc%"
toProfileKey=""

Review comment (suggestion):
```suggestion
    echo  "Usage: ./get-job-json.sh <jobname> <fromServer> <fromVersion> <toServer> <toVersion> <timeprefix> <actorprefix> <format> <issueNumber> <initBigRepo> <nomsBinFormat> <sysbenchTestTime> <withTpcc>"
```

Code after:
#!/bin/sh

set -e

if [ "$#" -lt 12 ]; then
    echo  "Usage: ./get-job-json.sh <jobname> <fromServer> <fromVersion> <toServer> <toVersion> <timeprefix> <actorprefix> <format> <issueNumber> <initBigRepo> <nomsBinFormat> <sysbenchTestTime> <withTpcc>"
    exit 1
fi

jobname="$1"
fromServer="$2"
fromVersion="$3"
toServer="$4"
toVersion="$5"
timeprefix="$6"
actorprefix="$7"
format="$8"
issueNumber="$9"
initBigRepo="${10}"
nomsBinFormat="${11}"
sysbenchTestTime="${12}"
withTpcc="${13}"
precision="2"
tpccRegex="tpcc%"
toProfileKey=""

if [ -n "$initBigRepo" ]; then
  initBigRepo="\"--init-big-repo=$initBigRepo\","
fi

if [ -n "$nomsBinFormat" ]; then
Repository: dotnet/aspnetcore
File: src/Components/WebAssembly/WebAssembly/src/Services/LazyAssemblyLoader.cs (C#)

Code before:
            return Array.Empty<Assembly>();
        }

        var loadedAssemblies = new List<Assembly>();
        var pendingLoads = newAssembliesToLoad.Select(assemblyToLoad => LoadAssembly(assemblyToLoad, loadedAssemblies));

        await Task.WhenAll(pendingLoads);
        return loadedAssemblies;
    }

    [RequiresUnreferencedCode("Types and members the loaded assemblies depend on might be removed")]
    [SupportedOSPlatform("browser")]
    private async Task LoadAssembly(string assemblyToLoad, List<Assembly> loadedAssemblies)
    {
        JSObject files = await LazyAssemblyLoaderInterop.LoadLazyAssembly(assemblyToLoad);

        byte[] dllBytes = files.GetPropertyAsByteArray("dll")!;
        byte[]? pdbBytes = files.GetPropertyAsByteArray("pdb");
        Assembly loadedAssembly = pdbBytes == null
                    ? AssemblyLoadContext.Default.LoadFromStream(new MemoryStream(dllBytes))
                    : AssemblyLoadContext.Default.LoadFromStream(new MemoryStream(dllBytes), new MemoryStream(pdbBytes));

        loadedAssemblies.Add(loadedAssembly);
        _loadedAssemblyCache!.Add(assemblyToLoad);

        files.Dispose();
    }

    private partial class LazyAssemblyLoaderInterop
    {
        [JSImport("Blazor._internal.loadLazyAssembly", "blazor-internal")]
        public static partial Task<JSObject> LoadLazyAssembly(string assemblyToLoad);
    }
}


Review comment (suggestion):
Nit (if earlier `using var` suggestion is applied):
```suggestion
```

Code after:
            return Array.Empty<Assembly>();
        }

        var loadedAssemblies = new List<Assembly>();
        var pendingLoads = newAssembliesToLoad.Select(assemblyToLoad => LoadAssembly(assemblyToLoad, loadedAssemblies));

        await Task.WhenAll(pendingLoads);
        return loadedAssemblies;
    }

    [RequiresUnreferencedCode("Types and members the loaded assemblies depend on might be removed")]
    [SupportedOSPlatform("browser")]
    private async Task LoadAssembly(string assemblyToLoad, List<Assembly> loadedAssemblies)
    {
        using var files = await LazyAssemblyLoaderInterop.LoadLazyAssembly(assemblyToLoad);

        var dllBytes = files.GetPropertyAsByteArray("dll")!;
        var pdbBytes = files.GetPropertyAsByteArray("pdb");
        Assembly loadedAssembly = pdbBytes == null
                    ? AssemblyLoadContext.Default.LoadFromStream(new MemoryStream(dllBytes))
                    : AssemblyLoadContext.Default.LoadFromStream(new MemoryStream(dllBytes), new MemoryStream(pdbBytes));

        loadedAssemblies.Add(loadedAssembly);
        _loadedAssemblyCache!.Add(assemblyToLoad);

    }

    private partial class LazyAssemblyLoaderInterop
    {
        [JSImport("Blazor._internal.loadLazyAssembly", "blazor-internal")]
        public static partial Task<JSObject> LoadLazyAssembly(string assemblyToLoad);
    }
}
Repository: dotnet/aspnetcore
File: src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyCultureProvider.cs (C#)

Code before:
        AssemblyLoadContext.Default.LoadFromStream(stream);
        wrapper.Dispose();
    }

    internal static string[] GetCultures(CultureInfo cultureInfo)
    {
        var culturesToLoad = new List<string>();

        // Once WASM is ready, we have to use .NET's assembly loading to load additional assemblies.
        // First calculate all possible cultures that the application might want to load. We do this by
        // starting from the current culture and walking up the graph of parents.
        // At the end of the the walk, we'll have a list of culture names that look like
        // [ "fr-FR", "fr" ]
        while (cultureInfo != null && cultureInfo != CultureInfo.InvariantCulture)
        {
            culturesToLoad.Add(cultureInfo.Name);

            if (cultureInfo.Parent == cultureInfo)
            {
                break;
            }

            cultureInfo = cultureInfo.Parent;
        }

        return culturesToLoad.ToArray();
    }

    private partial class WebAssemblyCultureProviderInterop
    {
        [JSImport("Blazor._internal.loadSatelliteAssemblies", "blazor-internal")]
        public static partial Task<JSObject> LoadSatelliteAssemblies(string[] culturesToLoad,
        [JSMarshalAs<JSType.Function<JSType.Object>>] Action<JSObject> assemblyLoader);
    }

}


Review comment (performance):
Curious about the reasoning behind this change - can `List<>`s not be marshalled? Or is it more efficient to marshal an array?

Code after:
        AssemblyLoadContext.Default.LoadFromStream(stream);
        wrapper.Dispose();
    }

    internal static string[] GetCultures(CultureInfo cultureInfo)
    {
        var culturesToLoad = new List<string>();

        // Once WASM is ready, we have to use .NET's assembly loading to load additional assemblies.
        // First calculate all possible cultures that the application might want to load. We do this by
        // starting from the current culture and walking up the graph of parents.
        // At the end of the the walk, we'll have a list of culture names that look like
        // [ "fr-FR", "fr" ]
        while (cultureInfo != null && cultureInfo != CultureInfo.InvariantCulture)
        {
            culturesToLoad.Add(cultureInfo.Name);

            if (cultureInfo.Parent == cultureInfo)
            {
                break;
            }

            cultureInfo = cultureInfo.Parent;
        }

        return culturesToLoad.ToArray();
    }

    private partial class WebAssemblyCultureProviderInterop
    {
        [JSImport("Blazor._internal.loadSatelliteAssemblies", "blazor-internal")]
        public static partial Task<JSObject> LoadSatelliteAssemblies(string[] culturesToLoad,
        [JSMarshalAs<JSType.Function<JSType.Object>>] Action<JSObject> assemblyLoader);
    }
}
Repository: dotnet/aspnetcore
File: src/Components/Endpoints/src/FormMapping/Converters/NullableConverter.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.

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;

internal sealed class NullableConverter<T>(FormDataConverter<T> nonNullableConverter) : FormDataConverter<T?>, ISingleValueConverter<T?> where T : struct
{
    private readonly FormDataConverter<T> _nonNullableConverter = nonNullableConverter;

    public bool CanConvertSingleValue() => _nonNullableConverter is ISingleValueConverter<T> singleValueConverter &&
        singleValueConverter.CanConvertSingleValue();

    public bool TryConvertValue(ref FormDataReader reader, string value, out T? result)
    {
        var converter = (ISingleValueConverter<T>)_nonNullableConverter;

        if (converter.TryConvertValue(ref reader, value, out var converted))
        {
            result = converted;
            return true;
        }
        else
        {
            result = null;
            return false;
        }
    }

    [RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
    [RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
    internal override bool TryRead(ref FormDataReader context, Type type, FormDataMapperOptions options, out T? result, out bool found)
    {
        if (!(_nonNullableConverter.TryRead(ref context, type, options, out var innerResult, out found) && found))
        {
            result = null;
            return false;
        }
        else
        {
            result = innerResult;
            return true;
        }
    }
}


Review comment (suggestion):
Instead of using a null suppression here, perhaps we should add [NotNullWhen] annotations to the `FormDataReader.TryGetValue` method.

Code after:
            result = converted;
            return true;
        }
        else
        {
            result = null;
            return false;
        }
    }

    [RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
    [RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
    internal override bool TryRead(ref FormDataReader reader, Type type, FormDataMapperOptions options, out T? result, out bool found)
    {
        // Donot call non-nullable converter's TryRead method, it will fail to parse empty
        // string. Call the TryConvertValue method above (similar to ParsableConverter) so
        // that it can handle the empty string correctly
        found = reader.TryGetValue(out var value);
        if (!found)
        {
            result = default;
            return true;
        }
        else
        {
            return TryConvertValue(ref reader, value!, out result!);
        }
    }
}
Repository: dotnet/aspnetcore
File: src/SignalR/server/Core/src/DynamicHub.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 Microsoft.AspNetCore.SignalR;

/// <summary>
/// A base class for SignalR hubs that use <c>dynamic</c> to represent client invocations.
/// </summary>
public abstract class DynamicHub : Hub
{
    private DynamicHubClients? _clients;

    /// <summary>
    /// Gets or sets an object that can be used to invoke methods on the clients connected to this hub.
    /// </summary>
    public new DynamicHubClients Clients
    {
        get
        {
            if (_clients == null)
            {
                _clients = new DynamicHubClients(base.Clients);
            }

            return _clients;
        }
        set => _clients = value;
    }
}


Review comment (suggestion):
```suggestion
using System.Diagnostics.CodeAnalysis;

namespace Microsoft.AspNetCore.SignalR;
```

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.Diagnostics.CodeAnalysis;

namespace Microsoft.AspNetCore.SignalR;

/// <summary>
/// A base class for SignalR hubs that use <c>dynamic</c> to represent client invocations.
/// </summary>
[RequiresDynamicCode("DynamicHub requires dynamic code generation to construct a call site.")]
public abstract class DynamicHub : Hub
{
    private DynamicHubClients? _clients;

    /// <summary>
    /// Gets or sets an object that can be used to invoke methods on the clients connected to this hub.
    /// </summary>
    public new DynamicHubClients Clients
    {
        get
        {
            if (_clients == null)
            {
                _clients = new DynamicHubClients(base.Clients);
            }

            return _clients;
        }
        set => _clients = value;
    }
Repository: dotnet/aspnetcore
File: src/SignalR/server/Core/src/DynamicHub.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 Microsoft.AspNetCore.SignalR;

/// <summary>
/// A base class for SignalR hubs that use <c>dynamic</c> to represent client invocations.
/// </summary>
public abstract class DynamicHub : Hub
{
    private DynamicHubClients? _clients;

    /// <summary>
    /// Gets or sets an object that can be used to invoke methods on the clients connected to this hub.
    /// </summary>
    public new DynamicHubClients Clients
    {
        get
        {
            if (_clients == null)
            {
                _clients = new DynamicHubClients(base.Clients);
            }

            return _clients;
        }
        set => _clients = value;
    }
}


Review comment (refactor):
```suggestion
[RequiresDynamicCode("DynamicHub requires dynamic code generation to construct a call site.")]
public abstract class DynamicHub : Hub
```

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.Diagnostics.CodeAnalysis;

namespace Microsoft.AspNetCore.SignalR;

/// <summary>
/// A base class for SignalR hubs that use <c>dynamic</c> to represent client invocations.
/// </summary>
[RequiresDynamicCode("DynamicHub requires dynamic code generation to construct a call site.")]
public abstract class DynamicHub : Hub
{
    private DynamicHubClients? _clients;

    /// <summary>
    /// Gets or sets an object that can be used to invoke methods on the clients connected to this hub.
    /// </summary>
    public new DynamicHubClients Clients
    {
        get
        {
            if (_clients == null)
            {
                _clients = new DynamicHubClients(base.Clients);
            }

            return _clients;
        }
        set => _clients = value;
    }
}
Repository: dotnet/aspnetcore
File: src/SignalR/server/Core/src/DynamicHubClients.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.

using Microsoft.AspNetCore.SignalR.Internal;

namespace Microsoft.AspNetCore.SignalR;

/// <summary>
/// A class that provides <c>dynamic</c> access to connections, including the one that sent the current invocation.
/// </summary>
public class DynamicHubClients
{
    private readonly IHubCallerClients _clients;

    /// <summary>
    /// Initializes a new instance of the <see cref="DynamicHubClients"/> class.
    /// </summary>
    /// <param name="clients">A wrapped <see cref="IHubCallerClients"/> that is used to invoke methods.</param>
    public DynamicHubClients(IHubCallerClients clients)
    {
        _clients = clients;
    }

    /// <summary>
    /// Gets an object that can be used to invoke methods on all clients connected to the hub.
    /// </summary>
    /// <returns>An object that can be used to invoke methods on the specified user.</returns>
    public dynamic All => new DynamicClientProxy(_clients.All);


Review comment (suggestion):
```suggestion
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.SignalR.Internal;
```

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.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.SignalR.Internal;

namespace Microsoft.AspNetCore.SignalR;

/// <summary>
/// A class that provides <c>dynamic</c> access to connections, including the one that sent the current invocation.
/// </summary>
[RequiresDynamicCodeAttribute("DynamicHubClients requires dynamic code generation to construct a call site.")]
public class DynamicHubClients
{
    private readonly IHubCallerClients _clients;

    /// <summary>
    /// Initializes a new instance of the <see cref="DynamicHubClients"/> class.
    /// </summary>
    /// <param name="clients">A wrapped <see cref="IHubCallerClients"/> that is used to invoke methods.</param>
    public DynamicHubClients(IHubCallerClients clients)
    {
        _clients = clients;
    }

    /// <summary>
    /// Gets an object that can be used to invoke methods on all clients connected to the hub.
    /// </summary>
    /// <returns>An object that can be used to invoke methods on the specified user.</returns>
Repository: dotnet/aspnetcore
File: src/SignalR/server/Core/src/Internal/DynamicClientProxy.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.

using System.Dynamic;

namespace Microsoft.AspNetCore.SignalR.Internal;

internal sealed class DynamicClientProxy : DynamicObject
{
    private readonly IClientProxy _clientProxy;

    public DynamicClientProxy(IClientProxy clientProxy)
    {
        _clientProxy = clientProxy;
    }

    public override bool TryInvokeMember(InvokeMemberBinder binder, object?[]? args, out object? result)
    {
        result = _clientProxy.SendCoreAsync(binder.Name, args!);
        return true;
    }
}


Review comment (suggestion):
```suggestion
using System.Diagnostics.CodeAnalysis;
using System.Dynamic;
```

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.Diagnostics.CodeAnalysis;
using System.Dynamic;

namespace Microsoft.AspNetCore.SignalR.Internal;

internal sealed class DynamicClientProxy : DynamicObject
{
    private readonly IClientProxy _clientProxy;

    [RequiresDynamicCodeAttribute("This constructor requires dynamic code generation to construct a call site.")]
    public DynamicClientProxy(IClientProxy clientProxy)
    {
        _clientProxy = clientProxy;
    }

    public override bool TryInvokeMember(InvokeMemberBinder binder, object?[]? args, out object? result)
    {
        result = _clientProxy.SendCoreAsync(binder.Name, args!);
        return true;
    }
}
Repository: dotnet/aspnetcore
File: src/SignalR/server/Core/src/Internal/DynamicClientProxy.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.

using System.Dynamic;

namespace Microsoft.AspNetCore.SignalR.Internal;

internal sealed class DynamicClientProxy : DynamicObject
{
    private readonly IClientProxy _clientProxy;

    public DynamicClientProxy(IClientProxy clientProxy)
    {
        _clientProxy = clientProxy;
    }

    public override bool TryInvokeMember(InvokeMemberBinder binder, object?[]? args, out object? result)
    {
        result = _clientProxy.SendCoreAsync(binder.Name, args!);
        return true;
    }
}


Review comment (suggestion):
```suggestion
    [RequiresDynamicCodeAttribute("This constructor requires dynamic code generation to construct a call site.")]
    public DynamicClientProxy(IClientProxy clientProxy)
```

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.Diagnostics.CodeAnalysis;
using System.Dynamic;

namespace Microsoft.AspNetCore.SignalR.Internal;

internal sealed class DynamicClientProxy : DynamicObject
{
    private readonly IClientProxy _clientProxy;

    [RequiresDynamicCodeAttribute("This constructor requires dynamic code generation to construct a call site.")]
    public DynamicClientProxy(IClientProxy clientProxy)
    {
        _clientProxy = clientProxy;
    }

    public override bool TryInvokeMember(InvokeMemberBinder binder, object?[]? args, out object? result)
    {
        result = _clientProxy.SendCoreAsync(binder.Name, args!);
        return true;
    }
}