[PATCH] D107350: Add a trait to Sequence to iterate enums

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added a comment.

Several observations:

1. It is entirely possible to hijack the system by providing the macro where you actually need to iterate the enum (see `llvm/tools/llvm-exegesis/lib/X86/Target.cpp`). That is even if the enum is not iterable, adding the macro in a different file will work fine.
2. One of the desirable property was to be able to mark the `enum` as iterable next to its definition. I wasn't able to do so for any of the enums I tagged. The macro usually sits at the end of the file (far from its definition) because the enums are nested in classes and you can't specialize the template here.
3. This design actually forces to `#include "llvm/ADT/Sequence.h"` in any file that declares an iterable enum even though there's no direct need to iterate the enum.
4. In the tests, I had to change the structure of the code completely to be able to make it work. It is likely that this is going to be difficult to use in a different codebase where the root namespace is not `llvm`.

The existence of 1. suggests that clients will declare the macro where they need it, bypassing the security mechanism.
As-is, I fear it doesn't bring a lot of value, @kuhar WDYT? Any suggestions on how to make it nicer to use / safer?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107350/new/

https://reviews.llvm.org/D107350