Use native-sized integers to avoid overhead for 32-bit platforms

[kpreisser]

Proposed changes

• Resolve TODOs in the TaskDialog code by using native-sized integers (nint/nuint) where applicable, to reduce overhead for 32-bit platforms and to avoid having to use the unsafe keyword.
• In class PARAM, cast IntPtr to nint instead of long where applicable.

Customer Impact

• None

Regression?

• No

Risk

Test methodology

• Testing the changed code running as x86 and x64 with various inputs, including values where an overflow could happen, to make sure it behaves the same as the previous code.
• Added tests for PARAM.

Test environment(s)

.NET SDK (reflecting any global.json):
Version: 6.0.100-alpha.1.20472.11
Commit: e55929c5a5

Runtime Environment:
OS Name: Windows
OS Version: 10.0.18363
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\6.0.100-alpha.1.20472.11\

Host (useful for support):
Version: 6.0.0-alpha.1.20468.7
Commit: a820ca1c4f

.NET SDKs installed:
5.0.100-rc.1.20452.10 [C:\Program Files\dotnet\sdk]
6.0.100-alpha.1.20472.11 [C:\Program Files\dotnet\sdk]

Microsoft Reviewers: Open in CodeFlow

[kpreisser]

It seems the build uses an older C# compiler (pre-16.7) that doesn't yet support the native-sized integers feature.

[kpreisser]

#3959 updated the SDK to 5.0.100-rc.1, so this builds successfully now.

[codecov]

Codecov Report

Merging #3603 (ae690c7) into master (36f3feb) will not change coverage.
The diff coverage is n/a.

@@              Coverage Diff              @@
##              master       #3603   +/-   ##
=============================================
  Coverage   97.96958%   97.96958%           
=============================================
  Files            499         499           
  Lines         259010      259010           
  Branches        4569        4569           
=============================================
  Hits          253751      253751           
  Misses          4457        4457           
  Partials         802         802           

Flag	Coverage Δ
Debug	97.96958% <ø> (ø)
production	100.00000% <ø> (ø)
test	97.96958% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[RussKie]

We'll review this, once we over the release hump.

[RussKie]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[tannergooding]

I believe the following should be sufficient:

Suggested change

	=> unchecked((nint)(nuint)(uint)ToInt(low, high));
	=> unchecked((nint)(uint)ToInt(low, high));

Due to converting from uint to nint, C# will just emit a conv.u which is a zero extension: https://sharplab.io/#v2:EYLgtghgzgLgpgJwDQxASwDYB8ACAmARgFgAoHAZgAJ9KBhSgb1MpeqoEkA7GABRgUoBZAgAo03SgDcIGAK5wAlI2atVOAOyVZnAMYALODoDWcACYiRncTAWXZ12/e4LpcxQG4VLAL5fKfikouXn4hPDEJV3klJhJVNU1tfUMTc0sHEScbKI8/XxJvIA

[weltkante]

@tannergooding well, its not readable, you don't know whether (nint)(uint) means uint -> int -> nint or uint -> nuint -> nint - see discussion above - and in fact for constants Roslyn was not doing zero-extension for a while.

[tannergooding]

It should be no different, logically, from doing a cast of uint to int (if a 32-bit process) or uint to long (if a 64-bit process), I don't think you would write (long)(ulong)(uint)value or (int)(uint)(uint)value so I (personally) wouldn't expect to see (nint)(nuint)(uint)value here either.

and in fact for constants Roslyn was not doing zero-extension for a while.

AFAIK, all these cases have been fixed. There were a few edge cases in the initial preview, and all of those that were made known have been fixed.
Going through a few cases, I didn't see any issues with constants when comparing nint/nuint to int/uint in RC2 (I did not check if they had all made it into RC1).

[weltkante]

My point is that its not readable because the operation does two things at a time and conceptually is ambigous, changing signed-ness and size of the number, and different programming languages spec differently what happens. C# has (apparently) specced it to zero extend, i.e. first widen then change sign. This is not obvious when reading a (nint)(uint) cast in source, it could just as well first change sign then widen, sign-extending in the process.

[tannergooding]

LGTM.

[RussKie]

Thank you all