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

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

In D107350#2937011 <https://reviews.llvm.org/D107350#2937011>, @gchatelet wrote:

> Thx for the patch.
> I gave it a try and it seems it does not work in the context of the tests because of "warning as errors".
>
>   /redacted/git/llvm-project/llvm/unittests/ADT/SequenceTest.cpp:210:1: error: unused function 'add_enum_traits' [-Werror,-Wunused-function]
>   DECLARE_ITERABLE_ENUM(UntypedEnum)
>   ^
>   /redacted/git/llvm-project/llvm/include/llvm/ADT/Sequence.h:52:15: note: expanded from macro 'DECLARE_ITERABLE_ENUM'
>     inline auto add_enum_traits(ENUM value) {                                    \
>                 ^
>
> Unfortunately marking the function as `static` will not work when the function is declared in a class (`static` has a different meaning there)
>
>   /redacted/git/llvm-project/llvm/include/llvm/Support/MachineValueType.h:1406:12: error: 'static' is invalid in friend declarations
>       friend DECLARE_ITERABLE_ENUM(MVT::SimpleValueType)
>              ^
>   /redacted/git/llvm-project/llvm/include/llvm/ADT/Sequence.h:52:3: note: expanded from macro 'DECLARE_ITERABLE_ENUM'
>     static inline auto add_enum_traits(ENUM value) {                                    \
>     ^
>
> The idea is interesting though, maybe there's a way to make it work? We could use different macros depending on context but ideally a single one would be best.

I did not build with warnings as errors and didn't realize this was a problem. I think we should be able to add an `LLVM_ATTRIBUTE_UNUSED` to the functions: https://clang.llvm.org/docs/AttributeReference.html#id363.

> AFAIU the proposal is about making sure that we don't iterate on an enum by mistake but in your patch it seems that calling `make_iterable_enum` bypasses this security (i.e. X86::CondCode has never been declared iterable <https://reviews.llvm.org/D107378#change-DgLEQ1SuLFJw> ). Was that intended?

Yes, it was intentional. This is the 'escape hatch' that I mentioned. This is so that it's still possible to iterate over enums when the declaration is missing, for example, when we cannot modify the code where the enum comes from (think an external library), or know that only a part of enum values is iterable and don't want to declare the whole thing as safe to iterate over.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107350