loader: re-enable stricter clang-tidy checks, fix findings, add regre… - #2020
Conversation
|
Author jpr42 not on autobuild list. Waiting for curator authorization before starting CI build. |
1 similar comment
|
Author jpr42 not on autobuild list. Waiting for curator authorization before starting CI build. |
|
CI Vulkan-Loader build queued with queue ID 91125. |
|
CI Vulkan-Loader build # 3725 running. |
|
CI Vulkan-Loader build # 3725 passed. |
charles-lunarg
left a comment
charles-lunarg
left a comment
There was a problem hiding this comment.
Only issue I have is the reorganization of loader_instance, which in some places makes reading the struct harder (by moving related members far apart).
I do wish diff's were smarter w.r.t removing else clauses, cause that made a lot of noise for what is a minor change.
| bool create_terminator_invalid_extension; | ||
| bool supports_get_dev_prop_2; | ||
|
|
||
| struct loader_instance_extension_enable_list enabled_extensions; |
There was a problem hiding this comment.
This is one place where I'd rather be much more selective as some of the variables are grouped together by usage, and this change reorganizes them by type.
For example:
uint32_t total_gpu_count;
uint32_t phys_dev_count_term;
struct loader_physical_device_term **phys_devs_term;
uint32_t phys_dev_count_tramp;
struct loader_physical_device_tramp **phys_devs_tramp;
```
The counts are now far away from the pointers they are suppose to be the count for.
This is as much an indication that there needs to be better structs to organize everything (ie, use a bespoke struct for the list of physical device trampolines), but for now I'd rather not lose context by rearranging members.
Grouping-by-usage (counts next to the pointers/arrays they describe) is more readable than grouping-by-type for padding, so keep the original layout and suppress the Padding finding for just this struct rather than disabling the check repo-wide.
|
Author jpr42 not on autobuild list. Waiting for curator authorization before starting CI build. |
1 similar comment
|
Author jpr42 not on autobuild list. Waiting for curator authorization before starting CI build. |
|
CI Vulkan-Loader build queued with queue ID 91279. |
|
CI Vulkan-Loader build # 3726 running. |
|
CI Vulkan-Loader build # 3726 passed. |
|
Thanks again! |
3 participants
…ssion tests