-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Audit/Update select
uses in DiagnosticXKinds.td to use enum_select
.
#123121
Comments
Hi! This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:
If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below. |
@llvm/issue-subscribers-good-first-issue Author: Erich Keane (erichkeane)
`enum_select` was added in this patch here: https://github.com//pull/122505
It is a really useful version of We need someone to audit all of our uses of So if the diagnostic tends to have a lot of One such example is from: #120327 , which is what encouraged this patch. Once we have ones identified, someone should then go through and convert these diagnostics. |
Hey! Kinda new here and would like to work on this. |
Great, have fun! Let me know if you have troubles and I'll help how I can! Also, add me to the pull request once you have one. |
No p! Could I get assigned? |
done |
@ayokunle321 still working on this? |
@amansharma612 Nah, would you like to work on it? |
Yes I can take a look |
enum_select
was added in this patch here: #122505It is a really useful version of
select
that also creates anenum
so that we don't have to use unreliable 'magic numbers'. However, the original patch touches one such diagnostic.We need someone to audit all of our uses of
select
and see if:1- the
select
has a significant number ofitems
in it.2- A lot of the uses of the
select
for the diagnostic in the source use a lot of magic numbers, OR an enum-made-just-for itSo if the diagnostic tends to have a lot of
/* ThingIWantSelected */ 5
sort of things (or worse, just the5
!), it is likely a good candidate. Ones that are 'calculated' based on some other criteria aren't a great.One such example is from: #120327 , which is what encouraged this patch.
Once we have ones identified, someone should then go through and convert these diagnostics.
The text was updated successfully, but these errors were encountered: