[PATCH] D39982: [IRBuilder] Set the insert point and debug location together
Vedant Kumar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 8 14:26:54 PST 2017
vsk added inline comments.
================
Comment at: include/llvm-c/Core.h:2832
LLVMBuilderRef LLVMCreateBuilder(void);
+/** Deprecated: Use LLVMPositionBuilderWithLoc instead. */
void LLVMPositionBuilder(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
----------------
whitequark wrote:
> vsk wrote:
> > whitequark wrote:
> > > Why should we deprecate these at all? It's legal to pass Loc = NULL for no location, right? Then why shouldn't we make e.g. `LLVMPositionBuilderBefore(...)` an alias to `LLVMPositionBuilderBeforeWithLoc(..., NULL)` and keep existing code working? Very few frontends in the wild bother with any debug information at all, I don't think it makes sense to break all of them.
> > We should deprecate these APIs because they aren't great for building frontends which do emit debug info, and because these APIs will (soon, hopefully) have no C++ counterparts.
> >
> > It's legal to pass Loc = NULL, but that rewrite is only behavior-preserving for frontends which emit no debug info. To make the transition easier for these frontends, I can provide an upgrade script which rewrites PositionBuilder* to PositionBuilder*WithLoc(..., NULL). Does that sound OK to you?
> No, it does not sound OK because most LLVM frontends aren't written in C, they just use the C interface through FFI. Such a change would be onerous for these frontends for no good reason. I am not convinced that "not having an exact C++ counterpart" is a valid reason for deprecating a C API either.
>
> I would be much more fine with plain out removing `SetCurrentDebugLocation` since that is used by fewer frontends.
I see only two alternatives to deprecating these C APIs: preserve their behavior or change it s.t they do not forward debug info. There's opposition to a silent behavior change on the mailing list and opposition to deprecating the APIs here.
That leaves the do nothing option: essentially, this patch sans the comments about deprecation. That's what I'll do. I'd be interested in any constructive alternatives you have.
https://reviews.llvm.org/D39982
More information about the llvm-commits
mailing list