Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
One issue, looks good otherwise. I don't know Go but it seems straightforward.
include/llvm-c/Comdat.h | ||
---|---|---|
24 ↗ | (On Diff #136969) | These should probably have explicit enumerator values so that they do not change over time. |
include/llvm-c/Comdat.h | ||
---|---|---|
24 ↗ | (On Diff #136969) | Happy to change, but I'd like to understand the reasoning for keeping the enum values immutable. In Comdat.cpp we map these values to the C++ equivalents, so from at least the LLVM perspective there's no problem with the values changing over time. Are we expecting other languages to hardcode the constants instead of using this C enum? |
include/llvm-c/Comdat.h | ||
---|---|---|
24 ↗ | (On Diff #136969) | I guess we could leave it alone. llvm-c/Core.h uses both numbered and unnumbered enums. It looks like when enumerators are removed, in practice people do transition the enum to something explicitly numbered. I guess the lesson is that maybe we can trust people to be careful when editing these enums, in which case, unnumbered is fine. |