Using the CsWin32 PInvoke library #7444

fernandanavamoya · 2022-07-19T00:07:29Z

Converted these threads:
-GetCurrentActCtx
-GetExitCodeThread
-GetThreadLocale
-GlobalFree

Fixed the information messages in:
-COM2PropertyDescriptor.cs
-Control.cs

Microsoft Reviewers: Open in CodeFlow

Converted these threads: -GetCurrentActCtx -GetExitCodeThread -GetThreadLocale -GlobalFree Fixed the information messages in: -COM2PropertyDescriptor.cs -Control.cs

fernandanavamoya · 2022-07-19T00:13:07Z

cc: @AArnott

src/System.Windows.Forms/src/System/Windows/Forms/AxHost.cs

Tanya-Solyanik · 2022-07-19T01:00:33Z

src/System.Windows.Forms.Primitives/src/NativeMethods.txt

@@ -8,3 +8,6 @@ GlobalUnlock
 GetCurrentProcessId
 GlobalReAlloc
 GlobalFree
+GetExitCodeThread


Is it possible to add a comment to this file that would explain what it is for?

The file format does support lines starting with // for comments. That said, I don't recommend it as it wouldn't sort properly and it would probably get outdated. Why not just search for the method name in the code (or use Find References) if you're curious how a method is used?

I'm curious how this file is used.

Any public documentation on this? Also, does it require explicit build of the project for source generators to trigger?

The documentation is at https://github.com/microsoft/cswin32 (the readme file).
This is a standard roslyn source generator. As such, it does not require an explicit build. In fact the generated source code is never saved to disk, but rather produced in memory during compilation.

It would be nice to add a link to this readme file to the top of the NativeMethods.txt file. If this is a single line comment, it wouldn't interfere with sorting file lines.

src/System.Windows.Forms.Primitives/src/NativeMethods.txt

RussKie · 2022-07-19T01:44:38Z

src/System.Windows.Forms.Primitives/src/System/Windows/Forms/Internals/ThemingScope.cs

            return s_contextCreationSucceeded
-                && Kernel32.GetCurrentActCtx(out IntPtr current).IsTrue()
-                && current == s_hActCtx;
+                && PInvoke.GetCurrentActCtx(&current) == true


~~Does PInvoke.GetCurrentActCtx(...) return a bool?~~
I checked and it returns BOOL, and this looks wonky...

The conversion is implicit

Suggested change

&& PInvoke.GetCurrentActCtx(&current) == true

&& PInvoke.GetCurrentActCtx(&current)

RussKie · 2022-07-19T01:57:52Z

src/System.Windows.Forms.Primitives/src/System/Windows/Forms/Internals/ThemingScope.cs

@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.

+using Windows.Win32;
+using Foundation = Windows.Win32.Foundation;


Why do we need this alias?

Ditto below

Foundation holds BOOL and Handle as well as other types.

Correct, but in this case it doesn't appear necessary as the screenshot shows. I suspect it's probably true for other instances in this PR too.

AArnott · 2022-07-19T04:48:38Z

src/System.Windows.Forms/src/System/Windows/Forms/AxHost.cs

@@ -3231,21 +3232,21 @@ public unsafe bool HasPropertyPages()

        private unsafe void ShowPropertyPageForDispid(Ole32.DispatchID dispid, Guid guid)
        {
-            IntPtr pUnk = Marshal.GetIUnknownForObject(GetOcx());
+            nint pUnk = Marshal.GetIUnknownForObject(GetOcx());


nit: I would leave this at IntPtr. You probably don't need the syntax support of nint (e.g. arithmetic), and IMO IntPtr is better for representing an opaque value like a pointer.
Similar feedback below. It looks pretty weird to cast pointers to nint.

In .NET 7 nint and IntPtr will refer to the exact same thing so changing between them does not make any difference.

AArnott · 2022-07-19T04:57:49Z

src/System.Windows.Forms/src/System/Windows/Forms/Control.cs

-                if ((returnValue.IsTrue() && exitCode != NativeMethods.STILL_ACTIVE) ||
-                    (returnValue.IsFalse() && Marshal.GetLastWin32Error() == ERROR.INVALID_HANDLE) ||
+                if ((returnValue == true && exitCode != NativeMethods.STILL_ACTIVE) ||
+                    (returnValue == false && Marshal.GetLastWin32Error() == ERROR.INVALID_HANDLE) ||


returnValue and !returnValue should work. No need to spell out == true and == false unless that's part of your codebase style guide, which at least for == true would be weird, IMO.

AArnott

Change looks good, although I would personally try to avoid mixing the syntax only changes (e.g. IntPtr to nint) with the CsWin32 changes.

Using the CsWin32 PInvoke library

02e958f

Converted these threads: -GetCurrentActCtx -GetExitCodeThread -GetThreadLocale -GlobalFree Fixed the information messages in: -COM2PropertyDescriptor.cs -Control.cs

fernandanavamoya requested a review from a team as a code owner July 19, 2022 00:07

ghost assigned fernandanavamoya Jul 19, 2022

dreddy-work reviewed Jul 19, 2022

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/AxHost.cs Show resolved Hide resolved

dreddy-work approved these changes Jul 19, 2022

View reviewed changes

Tanya-Solyanik approved these changes Jul 19, 2022

View reviewed changes

RussKie reviewed Jul 19, 2022

View reviewed changes

RussKie added the 📭 waiting-author-feedback The team requires more information from the author label Jul 19, 2022

elachlan mentioned this pull request Jul 19, 2022

Start using CsWin32 #7445

Closed

AArnott reviewed Jul 19, 2022

View reviewed changes

AArnott approved these changes Jul 19, 2022

View reviewed changes

RussKie mentioned this pull request Jul 19, 2022

Convert BitBlt to cswin32 PInvoke #7446

Merged

ghost removed the 📭 waiting-author-feedback The team requires more information from the author label Jul 19, 2022

dreddy-work added ready-to-merge PRs that are ready to merge but worth notifying the internal team. and removed ready-to-merge PRs that are ready to merge but worth notifying the internal team. labels Jul 20, 2022

RussKie added the 📭 waiting-author-feedback The team requires more information from the author label Jul 21, 2022

Merge branch 'feature/win32' into ThreadAPI18

7012ae0

ghost removed the 📭 waiting-author-feedback The team requires more information from the author label Jul 21, 2022

fernandanavamoya merged commit ec41384 into dotnet:feature/win32 Jul 21, 2022

fernandanavamoya deleted the ThreadAPI18 branch July 21, 2022 17:55

ghost locked as resolved and limited conversation to collaborators Aug 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using the CsWin32 PInvoke library #7444

Using the CsWin32 PInvoke library #7444

fernandanavamoya commented Jul 19, 2022 •

edited by ghost

Loading

fernandanavamoya commented Jul 19, 2022

Tanya-Solyanik Jul 19, 2022

AArnott Jul 19, 2022

Tanya-Solyanik Jul 19, 2022

dreddy-work Jul 19, 2022 •

edited

Loading

AArnott Jul 19, 2022

Tanya-Solyanik Jul 19, 2022

RussKie Jul 19, 2022 •

edited

Loading

RussKie Jul 19, 2022

RussKie Jul 19, 2022

elachlan Jul 19, 2022

RussKie Jul 19, 2022

AArnott Jul 19, 2022 •

edited

Loading

RussKie Jul 19, 2022

AArnott Jul 19, 2022

AArnott left a comment

	&& PInvoke.GetCurrentActCtx(&current) == true
	&& PInvoke.GetCurrentActCtx(&current)

Using the CsWin32 PInvoke library #7444

Using the CsWin32 PInvoke library #7444

Conversation

fernandanavamoya commented Jul 19, 2022 • edited by ghost Loading

Microsoft Reviewers: Open in CodeFlow

fernandanavamoya commented Jul 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dreddy-work Jul 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RussKie Jul 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AArnott Jul 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AArnott left a comment

Choose a reason for hiding this comment

fernandanavamoya commented Jul 19, 2022 •

edited by ghost

Loading

dreddy-work Jul 19, 2022 •

edited

Loading

RussKie Jul 19, 2022 •

edited

Loading

AArnott Jul 19, 2022 •

edited

Loading