Skip to content

ICorDebugProcess5.GetTypeFields - add missing MIDL attributes#131012

Open
MattParkerDev wants to merge 2 commits into
dotnet:mainfrom
MattParkerDev:patch-1
Open

ICorDebugProcess5.GetTypeFields - add missing MIDL attributes#131012
MattParkerDev wants to merge 2 commits into
dotnet:mainfrom
MattParkerDev:patch-1

Conversation

@MattParkerDev

Copy link
Copy Markdown

GetTypeFields seems to be missing MIDL attributes describing fields's size and length.

Copilot AI review requested due to automatic review settings July 18, 2026 06:48
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Jul 18, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the CoreCLR cordebug.idl COM interface definition to add MIDL annotations for ICorDebugProcess5::GetTypeFields, improving parameter direction/array sizing metadata for header/interop generation.

Changes:

  • Annotated celt as [in].
  • Annotated fields as [out] with size_is(...) / length_is(...) metadata.
  • Annotated pceltNeeded as [out].

Comment thread src/coreclr/inc/cordebug.idl Outdated
Copilot AI review requested due to automatic review settings July 18, 2026 07:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

HRESULT GetArrayLayout([in] COR_TYPEID id, [out] COR_ARRAY_LAYOUT *pLayout);
HRESULT GetTypeLayout([in] COR_TYPEID id, [out] COR_TYPE_LAYOUT *pLayout);
HRESULT GetTypeFields([in] COR_TYPEID id, ULONG32 celt, COR_FIELD fields[], ULONG32 *pceltNeeded);
HRESULT GetTypeFields([in] COR_TYPEID id, [in] ULONG32 celt, [out, size_is(celt), length_is(*pceltNeeded)] COR_FIELD fields[], [out] ULONG32 *pceltNeeded);
@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "90412d07cdcc39f4518ab88bc8d9203aafebddbe",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "3522269feadf7e6d9a5ed422ccd269335ebe62a9",
  "last_reviewed_commit": "90412d07cdcc39f4518ab88bc8d9203aafebddbe",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "3522269feadf7e6d9a5ed422ccd269335ebe62a9",
  "last_recorded_worker_run_id": "29688122321",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "90412d07cdcc39f4518ab88bc8d9203aafebddbe",
      "review_id": 4730808545
    }
  ]
}

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holistic Review

Motivation: ICorDebugProcess5::GetTypeFields in cordebug.idl was missing the MIDL directional and array-sizing attributes ([in], [out], size_is, length_is) that every other caller-allocated-buffer method in this interface uses. Without them, the generated proxy/stub marshalling cannot correctly describe the fields buffer, which matters for remote/out-of-process debugging scenarios and tooling that relies on the type library.

Approach: The single-line change annotates the parameters to match the well-established convention already used by sibling APIs such as EnumerateHeap, EnumerateHeapRegions, and GetGCReferences in the same file ([out, size_is(celt), length_is(*pceltNeeded)] on the array, [in] on the count, [out] on the needed-count pointer). This is exactly the pattern at lines 2498, 2556, and 2615, so the fix is consistent and idiomatic.

Summary: This is a correct, minimal, low-risk documentation/marshalling-correctness fix to an IDL declaration. The attributes accurately reflect the method's contract: celt is the input capacity, fields is a caller-allocated output buffer sized by celt with the valid element count given by *pceltNeeded, and pceltNeeded is an output. The change matches surrounding conventions and introduces no behavioral change to existing compiled clients (the underlying C++ signature is unchanged). No concerns; LGTM.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 30.5 AIC · ⌖ 9.96 AIC · ⊞ 10K

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Diagnostics-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants