Repository: dotnet/runtime
File: src/coreclr/vm/unsafeaccessors.cpp (C++)

Code before:
    TypeHandle firstArgType;
    CorElementType firstArgCorType = ELEMENT_TYPE_END;
    retCorType = context.DeclarationMetaSig.GetReturnType();
    retType = context.DeclarationMetaSig.GetRetTypeHandleThrowing();
    UINT argCount = context.DeclarationMetaSig.NumFixedArgs();
    if (argCount > 0)
    {
        context.DeclarationMetaSig.NextArg();

        // Get the target type signature and resolve to a type handle.
        context.TargetTypeSig = context.DeclarationMetaSig.GetArgProps();
        (void)context.TargetTypeSig.PeekElemType(&firstArgCorType);
        firstArgType = context.DeclarationMetaSig.GetLastTypeHandleThrowing();

        // For instance member access on value types, the first argument (this) must be a byref.
        // However, the target type used for token emission must not be parameterized. Keeping the
        // byref here can lead to incorrect shared generic context lookups at runtime.
        if ((context.Kind == UnsafeAccessorKind::Method || context.Kind == UnsafeAccessorKind::Field)
            && firstArgCorType == ELEMENT_TYPE_BYREF)
        {
            SigPointer targetTypeSig = context.TargetTypeSig;
            CorElementType elemType;
            IfFailThrow(targetTypeSig.GetElemType(&elemType));
            _ASSERTE(elemType == ELEMENT_TYPE_BYREF);

            context.TargetTypeSig = targetTypeSig;
        }
    }

    // Using the kind type, perform the following:
    //  1) Validate the basic type information from the signature.
    //  2) Resolve the name to the appropriate member.
    switch (context.Kind)
    {
    case UnsafeAccessorKind::Constructor:
        // A return type is required for a constructor, otherwise
        // we don't know the type to construct.
        // Types should not be parameterized (that is, byref).
        // The name is defined by the runtime and should be empty.
        if (context.DeclarationMetaSig.IsReturnTypeVoid() || retType.IsByRef() || !name.IsEmpty())
        {
            ThrowHR(COR_E_BADIMAGEFORMAT, BFA_INVALID_UNSAFEACCESSOR);
        }

        // Get the target type signature from the return type.
        context.TargetTypeSig = context.DeclarationMetaSig.GetReturnProps();
        context.TargetType = ValidateTargetType(retType, retCorType);
        if (!TrySetTargetMethod(context, ".ctor"))
            MemberLoader::ThrowMissingMethodException(context.TargetType.AsMethodTable(), ".ctor");
        break;


Review comment (refactor):
I must be missing some side-effect this code has. We extract `context.TargetTypeSig` call a function on the `SigPointer` and then assign it back immediately. The comment above provides details, but I don't see how they relate to this code.

Code after:
    TypeHandle firstArgType;
    CorElementType firstArgCorType = ELEMENT_TYPE_END;
    retCorType = context.DeclarationMetaSig.GetReturnType();
    retType = context.DeclarationMetaSig.GetRetTypeHandleThrowing();
    UINT argCount = context.DeclarationMetaSig.NumFixedArgs();
    if (argCount > 0)
    {
        context.DeclarationMetaSig.NextArg();

        // Get the target type signature and resolve to a type handle.
        context.TargetTypeSig = context.DeclarationMetaSig.GetArgProps();
        (void)context.TargetTypeSig.PeekElemType(&firstArgCorType);
        firstArgType = context.DeclarationMetaSig.GetLastTypeHandleThrowing();

        // For instance member access, the first argument is the receiver ("this").
        // For value types the receiver must be passed byref. A byref receiver can also occur for reference
        // types depending on the declaration signature.
        //
        // However, when emitting the metadata token for the accessed member, the owning type must not be
        // a byref. Keeping the byref here can lead to incorrect shared generic context lookups at runtime.
        //
        // No special handling is required for other forms of type parameterization here:
        //  * Arrays and generic instantiations are part of the owning type identity and must not be stripped.
        //  * Other forms (eg. pointers) are not valid target types and will be rejected by ValidateTargetType().
        if ((context.Kind == UnsafeAccessorKind::Method || context.Kind == UnsafeAccessorKind::Field)
            && firstArgCorType == ELEMENT_TYPE_BYREF)
        {
            // Consume byref token.
            (void)context.TargetTypeSig.GetElemType(nullptr);
        }
    }

    // Using the kind type, perform the following:
    //  1) Validate the basic type information from the signature.
    //  2) Resolve the name to the appropriate member.
    switch (context.Kind)
    {
    case UnsafeAccessorKind::Constructor:
        // A return type is required for a constructor, otherwise
        // we don't know the type to construct.
        // Types should not be parameterized (that is, byref).
        // The name is defined by the runtime and should be empty.
        if (context.DeclarationMetaSig.IsReturnTypeVoid() || retType.IsByRef() || !name.IsEmpty())
        {
            ThrowHR(COR_E_BADIMAGEFORMAT, BFA_INVALID_UNSAFEACCESSOR);
        }

        // Get the target type signature from the return type.
        context.TargetTypeSig = context.DeclarationMetaSig.GetReturnProps();
        context.TargetType = ValidateTargetType(retType, retCorType);
        if (!TrySetTargetMethod(context, ".ctor"))