Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work around false SLOTS defined but not used warning on GCC #10374

Merged
merged 1 commit into from May 9, 2022

Conversation

Pokechu22
Copy link
Contributor

This warning was caused by #10364; see e.g. this build log. The bug in question is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80351, and it seems to be solvable by explicitly using std::initializer_list instead of auto. The warning is generated only for the first auto initializer list, but we need to change both SLOTS and MEMCARD_SLOTS as if only SLOTS is changed, then MEMCARD_SLOTS becomes the first auto initializer list and generates a warning. An alternative solution is to mark SLOTS as [[maybe_unused]], which does not require changing MEMCARD_SLOTS, but that seems a bit silly. (I've requested an account on the GCC bugzilla to add some additional notes there, but it hasn't been created yet.)

@shuffle2
Copy link
Contributor

why not just explicitly state the type and make it a [] or std::array?

@Pokechu22
Copy link
Contributor Author

I don't think there's much of a benefit of using [] or std::array over explicitly stating that it's a std::initializer_list, but using either [] or std::array would require also listing the length (but I don't think it'd have any difference in performance or anything). auto deduces to std::initializer_list here, so there isn't any behavior difference from this change.

@shuffle2
Copy link
Contributor

[] doesn't require listing the length. the benefit is just that initializer_list is some new(er) jargon which apparently still has spotty support on some compilers. there should be no behavior difference in any case, but just using something like a plain old array seems simple and straightforward here (not that it really matters one way or the other, really - apart from the fact some workaround is needed).

@lioncash
Copy link
Member

It's definitely more idiomatic to use std::array for this sort of thing over an initializer_list. It's also shorter since with template deduction guides, you wouldn't need to state the size, you would just do:

constexpr std::array SLOTS = {Slot::A, Slot::B, Slot::SP1};

@Pokechu22
Copy link
Contributor Author

The freebsd builder doesn't like to deduce std::array, see https://dolphin.ci/#/builders/13/builds/5852

@lioncash
Copy link
Member

I think that's indicative more than anything that the freebsd builder needs to be updated, given that deduction guides have been supported by std lib implementations for quite a while now

@Pokechu22
Copy link
Contributor Author

For what it's worth, I've submitted a patch to GCC to fix this issue, and the patch is expected to land in GCC 13.

@Pokechu22
Copy link
Contributor Author

The patch has been merged, though it will be a while before it ends up in a release build of GCC (GCC 12 just entered the release candidate state, and this patch is for the GCC 13 branch).

@Tilka Tilka merged commit a768dc6 into dolphin-emu:master May 9, 2022
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants