[PATCH] D44086: [LLVM-C] [bindings/go] Add C and Golang bindings for COMDAT

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 13 13:39:17 PDT 2018


rnk accepted this revision.
rnk added inline comments.
This revision is now accepted and ready to land.


================
Comment at: include/llvm-c/Comdat.h:24
+typedef enum {
+  LLVMAnyComdatSelectionKind,        ///< The linker may choose any COMDAT.
+  LLVMExactMatchComdatSelectionKind, ///< The data referenced by the COMDAT must
----------------
ben-clayton wrote:
> rnk wrote:
> > These should probably have explicit enumerator values so that they do not change over time.
> 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?
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D44086





More information about the llvm-commits mailing list