Skip to content

Implement IXCLRDataTypeDefinition metadata methods in cDAC - #131365

Open
rcj1 with Copilot wants to merge 12 commits into
mainfrom
copilot/draft-implement-getcorelementtype-getname-gettoken
Open

Implement IXCLRDataTypeDefinition metadata methods in cDAC#131365
rcj1 with Copilot wants to merge 12 commits into
mainfrom
copilot/draft-implement-getcorelementtype-getname-gettoken

Conversation

Copilot AI commented Jul 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Implements cDAC-backed GetCorElementType, GetName, and GetTokenAndScope for IXCLRDataTypeDefinition.

Changes

  • Retain the module, type handle, and metadata token in ClrDataTypeDefinition.
  • Match native DAC naming, buffer, and HRESULT semantics.
  • Cross-validate cDAC results against the legacy DAC in debug builds.
  • Add coverage for outputs, invalid arguments, and buffer truncation.

Note

This description was generated by GitHub Copilot.

Copilot AI and others added 5 commits July 25, 2026 04:15
@rcj1
Implement cDAC type definition methods
515a501 
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
@rcj1
Test cDAC type definition methods
a706035 
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
@rcj1
Use native-compatible type names
5ef728c 
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
@rcj1
Guard cDAC debug output validation
f5180c4 
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
@rcj1
Harden cDAC debug result validation
140197d 
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 25, 2026 04:26
Copilot AI reviewed Jul 25, 2026

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@azure-pipelines

azure-pipelines Bot commented Jul 25, 2026

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

Copilot finished work on behalf of rcj1 July 25, 2026 04:27
Copilot AI requested a review from rcj1 July 25, 2026 04:27
@rcj1
Centralize insufficient-buffer HRESULT
2457d1e 
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 25, 2026 04:54
Copilot AI reviewed Jul 25, 2026

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@rcj1
Use shared HRESULT in cDAC test
19fcbae 
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 25, 2026 04:55
Copilot AI reviewed Jul 25, 2026

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot finished work on behalf of rcj1 July 25, 2026 04:57
@rcj1
Handle metadata-only type definitions
c5a3aa9 
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 25, 2026 06:06
Copilot AI reviewed Jul 25, 2026

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@rcj1
Complete null type-handle handling
096bc2d 
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 25, 2026 06:34
Copilot AI reviewed Jul 25, 2026

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot finished work on behalf of rcj1 July 25, 2026 06:36
@rcj1
Apply remaining changes
8180f7d 
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 26, 2026 05:22
Copilot AI reviewed Jul 26, 2026

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot finished work on behalf of rcj1 July 26, 2026 05:22
@dotnet-policy-service

dotnet-policy-service Bot commented Jul 26, 2026

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.

@build-analysis build-analysis Bot mentioned this pull request Jul 26, 2026
Open
@rcj1
rcj1 marked this pull request as ready for review July 27, 2026 18:08
@azure-pipelines

azure-pipelines Bot commented Jul 27, 2026

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

Copilot AI reviewed Jul 27, 2026
View reviewed changes

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.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 3

Comment thread src/native/managed/cdac/tests/UnitTests/TypeHandleTests.cs
Comment thread ...native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataTypeDefinition.cs
Comment thread src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CorDbHResults.cs
noahfalk
noahfalk approved these changes Jul 27, 2026
View reviewed changes
Comment thread ...native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataTypeDefinition.cs
}

#if DEBUG
if (LegacyFallbackHelper.CanFallback() && _legacyImpl is not null)

@noahfalk noahfalk Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (LegacyFallbackHelper.CanFallback() && _legacyImpl is not null)
if (_legacyImpl is not null)

Comment thread ...native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataTypeDefinition.cs
Comment thread ...native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataTypeDefinition.cs
}

#if DEBUG
if (LegacyFallbackHelper.CanFallback() && _legacyImpl is not null)

@noahfalk noahfalk Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (LegacyFallbackHelper.CanFallback() && _legacyImpl is not null)
if (_legacyImpl is not null)

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

Reviewers

@noahfalk noahfalk noahfalk approved these changes
Copilot code review Copilot Copilot left review comments
@rcj1 rcj1 Awaiting requested review from rcj1
@hoyosjs hoyosjs Awaiting requested review from hoyosjs
@max-charlamb max-charlamb Awaiting requested review from max-charlamb

Assignees

@rcj1 rcj1

Copilot code review Copilot

Labels

area-Diagnostics-cdac

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@noahfalk @rcj1