[llvm] [RemoveDIs] Add a 'BeforeDbgRecords' parameter to C API isnt insertion functions (PR #92417)

via llvm-commits llvm-commits at lists.llvm.org
Tue May 21 16:01:01 PDT 2024


alan-j-hu wrote:

- If `LLVMPositionBuilder` and `LLVMPositionBuilder2` are intended to co-exist - i.e., the latter is not intended to replace the former in any future release, I'm wondering if `LLVMPositionBuilder2` should have a more descriptive name. (The main case where I've seen the usage of `2` is the migration from typed to opaque pointers, where the old functions were eventually removed. Have there been any instances where a `2` function was added that didn't deprecate the older function?) For both the C and OCaml APIs, just adding a `2` at the end of the name doesn't indicate to the user what this function does differently from the other function.

- I would rather see `position_builder` implemented in OCaml code, not C FFI code. i.e.

  ```ocaml
  let position_builder ip = position_builder2 ip false
  ```

- Don't forget to replace the errorneous second declaration of `position_before` with the intended declaration of `position_before_dbg_records` in `llvm.mli`.

These are my only comments now; you have justified your design choices about `position_before` and `position_before_dbg_records`.

https://github.com/llvm/llvm-project/pull/92417


More information about the llvm-commits mailing list