vkd3d-shader: Minor fixes to clangd warnings (clangd).
Some fixes to valid clangd warnings, since I am using it as linter.
Also, I am removing enum hlsl_error_level on 4/6, since it doesn't seem to be meant to be used anywhere after 3/6.
Merge request reports
Activity
Subject: [PATCH 1/6] vkd3d-shader/tpf: Remove unnecessary fallthrough (clangd).
What is the issue here exactly? I don't necessarily have an issue with the change, but I wouldn't say it's an improvement purely on its own merits either.
#include "hlsl.h" +#include "vkd3d_shader.h" #include <stdio.h>
Is that needed? Doesn't hlsl.h pull in vkd3d_shader_private.h, which pulls in vkd3d_shader.h?
Subject: [PATCH 4/6] vkd3d-shader/hlsl: Remove enum hlsl_error_level.
We might as well merge that with the preceding patch that removes the last users of these.
Subject: [PATCH 1/6] vkd3d-shader/tpf: Remove unnecessary fallthrough (clangd).
What is the issue here exactly? I don't necessarily have an issue with the change, but I wouldn't say it's an improvement purely on its own merits either.Nothing really, besides that by default clangd emits a warning for all fall-throughs (that are not annotated in a rather specific way) so this warning shows in many places, but those in 1/5 and 5/5 can be seamlessly removed.
I agree that it is not important at all to go hunting for them, it just seemed like a natural think to do. Anyways, I joined 1/5 and 5/5 so it makes less noise. I can also remove them altogether.
#include "hlsl.h" +#include "vkd3d_shader.h" #include <stdio.h>
Is that needed? Doesn't hlsl.h pull in vkd3d_shader_private.h, which pulls in vkd3d_shader.h?
No, it is not needed. I may have added it by mistake.
Subject: [PATCH 4/6] vkd3d-shader/hlsl: Remove enum hlsl_error_level.
We might as well merge that with the preceding patch that removes the last users of these.
Done!
Edited by Francisco CasasNothing really, besides that by default clangd emits a warning for all fall-throughs (that are not annotated in a rather specific way) so this warning shows in many places, but those in 1/5 and 5/5 can be seamlessly removed.
This happens also on macOS (where the compiler is based on clang), and is the reason why we have
-Wno-implicit-fallthrough
inbuild-mac
for the CI. I would solve it properly rather than reorganize our code around that. It seems that both gcc and clang accept__attribute__((fallthrough));
as a fall through marker. So I would defined a macro calledVKD3D_FALLTHROUGH
that expands to that on gcc and clang and to nothing on other compilers. Then I'd remove-Wno-implicit-fallthrough
frombuild-mac
.I should add, though, that both proposed changes to avoid case fallthrough mostly make sense to me in their own, as they make the code a tad more readable, so I'm accepting the change anyway. My suggestion still stands for the other fallthrough cases in the code (mostly
ir.c
, I think), but doesn't have to be in this MR.
added 4 commits
- d5421bd6 - vkd3d-shader: Remove unnecessary fallthroughs (clangd).
- 82f8a641 - vkd3d-shader/tpf: Avoid translations to D3DDECLUSAGE and back (clangd).
- 485932cf - vkd3d-shader/hlsl: Remove enum hlsl_error_level (clangd).
- 581bb12c - vkd3d-shader/d3dbc: Use D3DSIO_TEXKILL instead of VKD3D_SM1_OP_TEXKILL (clangd).
Toggle commit listThis happens also on macOS (where the compiler is based on clang), and is the reason why we have
-Wno-implicit-fallthrough
inbuild-mac
for the CI. I would solve it properly rather than reorganize our code around that. It seems that both gcc and clang accept__attribute__((fallthrough));
as a fall through marker. So I would defined a macro calledVKD3D_FALLTHROUGH
that expands to that on gcc and clang and to nothing on other compilers. Then I'd remove-Wno-implicit-fallthrough
frombuild-mac
.So the history of -Wimplicit-fallthrough in vkd3d is that I was sceptical about introducing it in commit 8b49b6e0, but we only had a single function that needed adjustment, and the gcc mechanism for doing that was innocuous enough. I don't think that's true for the Clang mechanism, and adding a macro for it doesn't help much, in my opinion. My preferred way of addressing this issue would be to simply get rid of -Wimplicit-fallthrough from configure.ac.
That said, patch 1/4 would be fine with me if we also moved the default case in spirv_compiler_create() to the bottom of the switch.
So the history of -Wimplicit-fallthrough in vkd3d is that I was sceptical about introducing it in commit 8b49b6e0, but we only had a single function that needed adjustment, and the gcc mechanism for doing that was innocuous enough. I don't think that's true for the Clang mechanism, and adding a macro for it doesn't help much, in my opinion. My preferred way of addressing this issue would be to simply get rid of -Wimplicit-fallthrough from configure.ac.
I think
-Wimplicit-fallback
has some value, but I can live without it.
added 4 commits
- 38f1102d - vkd3d-shader: Remove unnecessary fallthroughs (clangd).
- 02ba54fc - vkd3d-shader/tpf: Avoid translations to D3DDECLUSAGE and back (clangd).
- a52b6f9a - vkd3d-shader/hlsl: Remove enum hlsl_error_level (clangd).
- 62dfd1ed - vkd3d-shader/d3dbc: Use D3DSIO_TEXKILL instead of VKD3D_SM1_OP_TEXKILL (clangd).
Toggle commit listadded 16 commits
-
62dfd1ed...6034f4a9 - 12 commits from branch
wine:master
- f0da419a - vkd3d-shader: Remove unnecessary fallthroughs (clangd).
- 432fa8fa - vkd3d-shader/tpf: Avoid translations to D3DDECLUSAGE and back (clangd).
- 79608365 - vkd3d-shader/hlsl: Remove enum hlsl_error_level (clangd).
- 38a73097 - vkd3d-shader/d3dbc: Use D3DSIO_TEXKILL instead of VKD3D_SM1_OP_TEXKILL (clangd).
Toggle commit list-
62dfd1ed...6034f4a9 - 12 commits from branch