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

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 3 00:49:00 PDT 2024


nikic wrote:

> For the intrinsic-inserting functions, do you think we should: a) mark as deprecated but otherwise leave them alone for now b) mark as deprecated and change their behaviour so that they do nothing and return nullptr c) outright delete them (it sounds like you're voting this one, but just wanted to double check)

I'd go for c), yes.

> * 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.

You are right that `2` usually indicates that the old function is deprecated -- though in some cases it is soft-deprecated without intent to remove (e.g. when we're just changing parameter types from unsigned to uint64_t or similar).

The naming pattern that often gets used for cases like this is "With", so we have both LLVMBuildCall2 and LLVMBuildCallWithOperandBundles, where the latter just provides additional functionality, but most use-cases can still use the former just fine. This is similar to the situation here.

So I guess you could call it LLVMPositionBuilderWithDbgRecordControl, which is, uh, quite the mouthful :) I don't really have strong opinions on the precise naming here.

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


More information about the llvm-commits mailing list