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

via llvm-commits llvm-commits at lists.llvm.org
Mon May 20 16:04:03 PDT 2024


https://github.com/alan-j-hu requested changes to this pull request.

First of all, you wrote a second declaration for `position_before` in `llvm.mli` where I assume you meant to declare `position_before_dbg_records`. Consequently, the tests do not pass.

Are `LLVMPositionBuilder` and `LLVMPositionBuilder2` intended to co-exist forever, or will the latter eventually deprecate the former?

If `LLVMPositionBuilder` will be deprecated, I would rather change `position_builder` right now to take the `bool`, and call the underlying C function `LLVMPositionBuilder2` than do the song and dance with the `2`. As far as I know, OCaml libraries don't have these kinds of ABI compatibility concerns, it's much better to rip off the band-aid once, between two major releases, than go through multiple steps of deprecation across multiple major releases.

Are `position_before` and `position_before_dbg_records` intended to co-exist, or will the latter eventually deprecate the former? By inserting after debug records, does `position_before` have "surprising" behavior that may cause errors?

If `position_before` has "surprising" behavior, or behavior that shouldn't be the "default," I would rather have it behave as `position_before_dbg_records`, i.e. inserting before debug records be the default action unless the programmer says otherwise.

I'm not an expert on the debug record migration, so please let me know if:

- If I didn't explain any of my suggestions clearly
- Any of my suggestions don't make sense
- Any of my suggestions may cause issues for users


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


More information about the llvm-commits mailing list