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.

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

API diff between .NET 9 Preview1 and .NET 9 Preview2 #9220

Merged
merged 5 commits into from
Mar 14, 2024

Conversation

carlossanlop
Copy link
Member

@carlossanlop carlossanlop commented Mar 12, 2024

Repo area owners:

Libraries area owners:

  • @dotnet/area-system-runtime
  • @dotnet/area-system-io
  • System.Net: @dotnet/ncl
  • @dotnet/area-system-numerics
  • @dotnet/area-system-runtime-intrinsics
  • @dotnet/area-system-security
  • @dotnet/area-system-text-json
  • System.Threading: @mangod9 @kouvel

Known api-diff tool issues:

If yo usee any of these, please provide a GitHub suggestion in this PR to correct it:

@@ -0,0 +1,13 @@
# System.Numerics
Copy link
Member

Choose a reason for hiding this comment

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

System.Numerics LGTM

@@ -0,0 +1,374 @@
# System.Runtime.Intrinsics.Arm
Copy link
Member

Choose a reason for hiding this comment

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

System.Runtime.Intrinsics.Arm LGTM

@@ -0,0 +1,86 @@
# System.Runtime.Intrinsics.X86
Copy link
Member

Choose a reason for hiding this comment

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

System.Runtime.Intrinsics.X86 LGTM

@@ -0,0 +1,31 @@
# System.Runtime.Intrinsics
Copy link
Member

Choose a reason for hiding this comment

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

System.Runtime.Intrinsics LGTM

Comment on lines +6 to +13
+ public static byte CompareExchange(ref byte location1, byte value, byte comparand);
+ public static short CompareExchange(ref short location1, short value, short comparand);
+ public static sbyte CompareExchange(ref sbyte location1, sbyte value, sbyte comparand);
+ public static ushort CompareExchange(ref ushort location1, ushort value, ushort comparand);
+ public static byte Exchange(ref byte location1, byte value);
+ public static short Exchange(ref short location1, short value);
+ public static sbyte Exchange(ref sbyte location1, sbyte value);
+ public static ushort Exchange(ref ushort location1, ushort value);
Copy link
Member

Choose a reason for hiding this comment

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

The unsigned overloads have an [System.CLSCompliantAttribute(false)] attribute on them, should that also be shown here? LGTM otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

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

If the attribute was freshly added or removed, it should've showed up in the diff. Was that the case?

These are the only attributes I'm explicitly excluding (by request from past api diff PRs):
https://github.com/carlossanlop/arcade/blob/AsmDiffUpdates/src/Microsoft.DotNet.AsmDiff/AttributesToExclude.txt

I opened this issue to track the request: dotnet/arcade#14585

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the APIs were added along with the attribute.

- public struct JsonReaderState
+ public readonly struct JsonReaderState
public sealed class JsonSerializerOptions {
+ public bool AllowOutOfOrderMetadataProperties { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

STJ changes LGTM.

@carlossanlop carlossanlop merged commit dc467dd into dotnet:main Mar 14, 2024
3 checks passed
@carlossanlop carlossanlop deleted the ApiDiffPrev2 branch March 14, 2024 01:52
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

System.IO is reporting something old as new.

``` diff
namespace System.IO {
public class FileStream : Stream {
+ public override void CopyTo(Stream destination, int bufferSize);
Copy link
Member

Choose a reason for hiding this comment

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

This is not new, the last time it was touched was 3 years ago:

image

Copy link
Member

Choose a reason for hiding this comment

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

Was it in the ref?

Copy link
Member

Choose a reason for hiding this comment

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

@stephentoub Yes, at least since since 2020:

image

BTW GH does not like the size of this file:

image

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

image

[Insert moral philosophy joke here]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants