AK: Simplify usage of windows.h and winsock2.h

[stasoid]

Use <AK/windows.h> instead of windows.h/winsock2.h to avoid timeval-related errors.

[ladybird-bot]

Hello!

One or more of the commit messages in this PR do not match the Ladybird code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

[R-Goc]

This is not like for like. Winsock isn't the same as windows. So if we name it windows.h we should be including windows.h. winsock is only for the sockets. Maybe we can include both in AK/Windows.h.

[stasoid]

winsock2.h includes windows.h, no need to include both.

[R-Goc]

Didn't know that. Then it's fine. Is there anything else we might want here?

[stasoid]

Maybe, we will know when we need that.

[R-Goc]

Any reason this starts with lower case specifically?

[stasoid]

Just my preference.

[R-Goc]

This doesn't match convention in AK, nor the normal name for it. Causes a compilation error due to non-portable-include if you write with uppercase.

[stasoid]

Not matching the AK convention is deliberate. I wanted this file to be different because it is a special platform header, it doesn't define any classes/functions in AK namespace. The same goes for sockaddr-win.h.

The "normal" name for <windows.h> debatable, but I think it is more lowercase than titlecase. MSDN examples use lowercase, Visual Studio templates use lowercase. I saw lowercase version much more often in real code. Also, this header is supposed to include everything from win32 API that is not included by other headers (like sockets and time), so it doesn't directly correspond to <windows.h> and doesn't have to follow its conventions.

But again, it is more a matter of preference. If maintainers make me change it I'll change it, but for now I want to keep lowercase.

[konradekk]

idea: be more restrictive with ideas like in the following?

Suggested change

	#ifdef AK_OS_WINDOWS // needed for Swift
	# define timeval dummy_timeval
	# include <winsock2.h>
	# undef timeval
	# include <io.h>
	#endif
	#ifndef AK_OS_WINDOWS
	static_assert(false, "Windows header included on non-Windows target");
	#endif
	// needed for Swift
	#define timeval dummy_timeval
	#include <winsock2.h>
	#undef timeval
	#include <io.h>

[ADKaster]

But again, it is more a matter of preference. If maintainers make me change it I'll change it, but for now I want to keep lowercase.

Consider this a maintainer request to change the names of any headers added in your PRs to Titlecase, regardless of how platform-specific they are. :)

[ADKaster]

The "needed for swift" comment is confusing. Have you tried enabling swift on windows?

[ADKaster]

If this is related to wanting to error out on other platforms if they include this header, the path to it will need updated in the ENABLE_SWIFT branch of AK/CMakeLists.txt to be an EXCLUDED path for the Swift bridging header. See LibCore for an example.

[stasoid]

I added #ifdef to fix this error. LibCore (or other libs) doesn't have the word EXCLUDED in their CMakeLists files. Can you provide the exact line that should be added to exclude Windows.h?

[stasoid]

Nvm, found it: set(SWIFT_EXCLUDE_HEADERS ... in LibGfx.

[stasoid]

Adding set(SWIFT_EXCLUDE_HEADERS Windows.h) to AK/CMakeLists.txt didn't fix the problem (same error), so I am still using #ifdef.

[stasoid]

@ADKaster I changed it to title case, but note that AK already has similar platform-specific files, kmalloc.h/.cpp, and they use lowercase. I think you are wrong on this one.

[stasoid]

@awesomekling Can you comment on this ^^^ ? (Platform-specific windows.h vs Windows.h in AK)

[trflynn89]

kmalloc.h is an ancient artifact from Serenity's kernel using AK. It is not a good argument to break naming conventions in new files.

[awesomekling]

@awesomekling Can you comment on this ^^^ ? (Platform-specific windows.h vs Windows.h in AK)

Please conform to project convention and name it Windows.h

[stasoid]

@ADKaster I renamed the file and added AK/Platform.h, but was unable to get rid of #ifdef.

[github-actions]

Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest master.