Merged
Conversation
Placing `alignas` before the type is more in-line with the C++ standard, Microsoft Learn examples, and other places in this codebase. While the old way worked with MSVC, this is more consistent and improves compatibility with other tools without affecting the MSVC build.
The forward declaration of InterpolationMode already included its underlying type. This commit does the same for ResourceRetType and ResourceDimension.
T::IsForwardReferenceable is a static boolean member, not a type.
Without the comma, this "igvk32.dll", "igvk64.dll" "igxelpgicd32.dll", "igxelpgicd64.dll", becomes "igvk32.dll", "igvk64.dlligxelpgicd32.dll", "igxelpgicd64.dll",
`CreateThread` expects `lpStartAddress` to be a function with the signature DWORD WINAPI ThreadProc( _In_ LPVOID lpParameter ); on Win32 `WINAPI` is `__stdcall`, but `CheckHookThread` previously had the effective signature DWORD __cdecl CheckHookThread(LPVOID param); because the calling convention was unspecified. The cast that was hiding this error is also removed.
Owner
|
This looks good to me. You are correct that I don't plan to support building with clang on windows but I'm fine accepting compiler warning fixes in general as long as they are correct & safe, which all of these are. The enum declaration thing was not intentional I don't believe and probably more just an inherited legacy from code before enums could have base types. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
While setting up clangd on Windows, I found a few errors/warnings to fix that might benefit the MSVC build. I tried to describe why they might be valuable outside of Clang in the commit messages, understanding that you have no intention of supporting Clang as a compiler on Windows. I'd be happy to drop any commits that aren't valuable fixes for an MSVC build or aren't actually errors.
I'm not 100% sure if the lack of underlying types "fixed" by Add enum underlying type was intentional or not. It's a little odd with the typedefs right under it seeing the pre-uint8_t enums and then the definitions with the underlying types in the other files. If it was intentional then a better fix might be namespacing like
enum MinimumPrecisionhas or letting MSVC do its thing and drop the commit. This change could also break old serialized files if either of these types are stored somewhere.Clang had thousands more warnings, but I'm definitely not the right person to evaluate which of those are worth addressing, so I won't waste your time with any more PRs. I only mention it on the off chance there's somebody else out there who might find joy sifting through that. Thank you for developing RenderDoc and sorry if this type of PR is unwelcome.