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

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: include/llvm/IR/IRBuilder.h:130
+  /// \deprecated{Please use:
+  ///   setInsertPoint(BasicBlock *, DebugLoc)
+  /// }
----------------
aprantl wrote:
> vsk wrote:
> > echristo wrote:
> > > 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.
> > Deprecating the existing API lets us switch to the new one incrementally, which would be less disruptive. If we don't mind some churn and would rather get this all done in one shot, I'm open to that (& can check in a script). @aprantl, @davide -- thoughts?
> I don't have a strong opinion. Whatever works best for you. If you are going to convert all uses anyway you might as do it in one go, or you can aim at smaller, incremental commits.
Well we need to provide an upgrade script anyway. And if we don't remove the old APIs now it's not clear when we will. I'm leaning towards updating everything in one shot.


https://reviews.llvm.org/D39982