[PATCH] D39982: [IRBuilder] Set the insert point and debug location together

[Eric Christopher via Phabricator via llvm-commits]

echristo added a comment.

I've made a couple of comments on API naming and deprecating inline.



================
Comment at: include/llvm-c/Core.h:2832
 LLVMBuilderRef LLVMCreateBuilder(void);
+/** Deprecated: Use LLVMPositionBuilder2 instead. */
 void LLVMPositionBuilder(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
----------------
I really dislike the "2" moniker on APIs. Seems that if we want a new API that has a new behavior we should do that instead with new and descriptive names.

LLVMPositionBuilderWithLoc?


================
Comment at: include/llvm/IR/IRBuilder.h:130
+  /// \deprecated{Please use:
+  ///   setInsertPoint(BasicBlock *, DebugLoc)
+  /// }
----------------
vsk wrote:
> davide wrote:
> > I'd really love to hear Eric (@echristo) input on this, but I have mixed feelings about deprecating an API and introducing a new API with the same name (and just a different overload).
> > It seems like people might still use this wrong, should we choose a different name?
> I'm open to changing the name here to make it more explicit, e.g setInsertPointAndDebugLoc.
Why are we deprecating a C++ API at all? Just change it.


https://reviews.llvm.org/D39982