[PATCH] D58733: [LLVM-C] Move and Clean Up The Instrumentation Pass Bindings

[whitequark via Phabricator via llvm-commits]

whitequark added inline comments.


================
Comment at: llvm/include/llvm-c/Instrumentation.h:45
+    LLVMBool NoPrune,
+    LLVMBool StackDepth);
+
----------------
I think enormous parameter lists are a very error-prone way to do this kind of binding. What about making a C structure reflecting the currently recognized options, and passing the length of this structure to the LLVMCreate*Options functions? This is similar to what WinAPI is doing.

I.e.
```
struct SanitizerCoverageOptions {
  unsigned CoverageType;
  LLVMBool IndirectCalls;
  // ...
}

LLVMSanitizerCoverageOptions LLVMCreateSanitizerCoverageOptions(
    SanitizerCoverageOptions *Options,
    unsigned OptionsLength
);
```

So, we could deprecate options by making them dead (and hopefully there's some attribute we can use to cause a warning to be emitted on access), and add new ones by appending them towards the end of the structure. For C/C++ users of the C API as well as any autogenerated bindings, the upgrades are now completely transparent. For non-autogenerated bindings, it's still less work to add new options.

@echristo @deadalnix What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58733