[PATCH] D93454: [LLVM-C] Replace LLVMSetInstDebugLocation with LLVMAddMetadataToInst.

[Tom Stellard via Phabricator via llvm-commits]

tstellar added a comment.

In D93454#3005834 <https://reviews.llvm.org/D93454#3005834>, @fhahn wrote:

> In D93454#3001992 <https://reviews.llvm.org/D93454#3001992>, @tstellar wrote:
>
>> This is an API break to the C API, which is allowed, but is there any way we can keep the old function?
>
> We could keep the old function and just have it call `AddMetadataToInst`. There should be no functional change *if* the user does not use the new functionality to collect non-debug metadata in the builder. Not sure if that's a great option. We could also extend `AddMetadataToInst` to accept a filter to only copy specific metadata kinds and have `LLVMAddMetadataToInst` use this filter. But this would also mean that we need to maintain a change `IRBuilder` which is only there for C API compatibility. In that case, we might as well keep the old `SetInstDebugLocation`. What do you think?

To me it sounds like keeping the old function would be best.  We can also deprecate it in the release notes so that users are aware that it might be removed at some point, and that it might not work with the new functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93454