[clang] [Clang][Diagnostics] Update select uses in DiagnosticXKinds.td to use enum_select (PR #130868)

[Erich Keane via cfe-commits]

erichkeane wrote:

> @erichkeane thanks a lot for the clarification. I think I have a hang of it now- I was kinda confused before. Now, my approach is to look through DiagnosticXKinds.td files -> check for a `select` with a significant number of items -> check its diagnostic ID -> grep the lib directory with this ID and find the uses of the diagnostic -> see if it’s up for replacement (caller has a lot of magic numbers/an enum made just for it).
>

Yeah, that seems like a good strategy.

> Now I wanted to ask - what do you consider a lot - 3 or more uses with magic numbers?

I don't have a 'hard' number here.  When I was looking for a few to demo it with, I just found ones where it was 'most' or 'all' uses.

> 
> For a `select` with a significant number of items, I’m thinking of going with 4 or more items as it seems reasonable. I was also looking through the codebase using this heuristic exposed a lot more for replacement (rather than 5 which I had in mind before).
> 

Another time I don't have a 'hard' number.  It is a bit of a balance between 'a lot of work' and 'transform enough of the uses that it improves readability'.  If 4 gives a good number of candidates, that is probably a good place to start.

> Also could you just brief me through the process of a change. I know the .td files are used to generate header files. So do I have to change the select to an enum_select first in the .td files and then build and then reference the generated enum option in the caller (e.g. diag::MemClassWork::AliasDecl) and then rebuild.

Yes, this would be the first/second step.

> The thing about this is that the build would probably fail the first time as the callers will still make use of the magic numbers. 

Magic numbers will still compile and work.  The version of `Diag`'s operator `<<` that this calls is still the one that takes an integral value.  So adding `enum_select` to something, as long as you don't change the order/number of elements, shouldn't break anything.

>I also found an enum declared just for a select that was already in a header file. For this case, I just remove this declaration right? And then replace the select. I guess I’m just confused about the whole header generation process - where the new enums would live since the relevant header files already exist before a build (e.g Sema.h, Decl.h) and since we’re making changes to .td files which are the header generators. Kinda new to clang stuff so forgive me for these questions 😅

Yes, you'd want to remove the old ones.  Tablegen will create the enums in the file `ClangDiagnostic${component}Enums.inc` I think, so search your build directory and you'll see where those are.


https://github.com/llvm/llvm-project/pull/130868