Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

C#: Update stubs #18587

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

C#: Update stubs #18587

wants to merge 9 commits into from

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Jan 24, 2025 •
edited
Loading

In this PR we

  • Update the generated stubs.
  • Add some manual models to some of the new params ReadOnlySpan<...> method overloads.

Added some extra projects due to prior breaking changes in .NET

@github-actions github-actions bot added the C# label Jan 24, 2025
@michaelnebel michaelnebel force-pushed the csharp/updatestubs branch 4 times, most recently from 7a5c30b to 98062c9 Compare January 27, 2025 12:37
@github-actions GitHub Actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",47,10819,54,5
+    System,"``System.*``, ``System``",47,10864,54,5
-    Totals,,108,12901,400,9
+    Totals,,108,12946,400,9
  • Changes to framework-coverage-csharp.csv:
- System,54,47,10819,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,5512,5307
+ System,54,47,10864,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,5547,5317

@michaelnebel
Copy link
Contributor Author

DCA looks good.

@michaelnebel michaelnebel marked this pull request as ready for review January 29, 2025 12:30
@Copilot Copilot bot review requested due to automatic review settings January 29, 2025 12:30
@michaelnebel michaelnebel requested a review from a team as a code owner January 29, 2025 12:30
Copilot
Copilot AI reviewed Jan 29, 2025
View reviewed changes

Choose a reason for hiding this comment

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

Copilot reviewed 369 out of 369 changed files in this pull request and generated no comments.

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
hvitved
hvitved reviewed Jan 30, 2025
View reviewed changes
csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected Outdated
@@ -3285,41 +3323,6 @@ summary
| Microsoft.VisualBasic;VBCodeProvider;VBCodeProvider;(System.Collections.Generic.IDictionary<System.String,System.String>);Argument[0].Element;Argument[this];taint;df-generated |
| Microsoft.Win32.SafeHandles;SafeFileHandle;SafeFileHandle;(System.IntPtr,System.Boolean);Argument[0];Argument[this].Field[System.Runtime.InteropServices.SafeHandle.handle];value;dfc-generated |
| Microsoft.Win32.SafeHandles;SafeWaitHandle;SafeWaitHandle;(System.IntPtr,System.Boolean);Argument[0];Argument[this].Field[System.Runtime.InteropServices.SafeHandle.handle];value;dfc-generated |
| Microsoft.Win32;PowerModeChangedEventHandler;BeginInvoke;(System.Object,Microsoft.Win32.PowerModeChangedEventArgs,System.AsyncCallback,System.Object);Argument[2];Argument[2].Parameter[delegate-self];value;hq-generated |
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it expected that we loose so many rows?

Copy link
Contributor Author

@michaelnebel michaelnebel Jan 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is because some of dependencies have changed such that System.Windows.Extensions is no longer referenced in the updated versions of the packages. If this is of concern, I can take a look and see whether se should try to explicitly include these packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added some extra dependencies. Maybe that is for the best. It will also be interesting to see what happens when we re-generate the models for .NET (for .NET 9).
Awaiting CI before setting ready for review again.

@michaelnebel michaelnebel marked this pull request as draft January 30, 2025 15:32
@michaelnebel michaelnebel marked this pull request as ready for review January 31, 2025 10:03
@michaelnebel michaelnebel requested a review from hvitved January 31, 2025 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@hvitved hvitved Awaiting requested review from hvitved

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
C# documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@michaelnebel @hvitved