[LLVM-C] Add C APIs to access DebugLoc info

[jberdine]

Summary

Add thin shims to C interface to provide access to DebugLoc info for
Instructions, GlobalVariables and Functions.

[whitequark]

Why are there three sets of functions that all operate on LLVMValueRef? Can't we make it a single set of functions?

All new C API functions that return strings must accept an unsigned *Length parameter.

Since this is a rather nontrivial addition to the C API, I think you should split this into a C patch which I'd like to get a second opinion on, and then a separate OCaml patch.

[jberdine]

split LLVM-C from OCaml changes

[jberdine]

I don't know if this is the idiomatic way to do dynamic dispatch in C, feedback very welcome.

I put these functions after the Metadata ones, but there might be a better place.

@whitequark, did you have someone in mind to look at these LLVM-C changes, or should I do some git blaming?

[whitequark]

@deadalnix, @echristo, any opinion on this C API?

[abdulras]

lib/IR/Core.cpp

1162

I think that unwrap(Val) should be possible to be hoisted rather than done in each case.

1163

Can't you hoist the unwrap here into the conditional?

if (const auto *I = unwrap<Instruction>(Val))

1164

Please mark the Length parameter as _Nonnull, add an assert, or add a check for Length being non-null here.

1176

I think that this is better with an early return:

if (GVEs.size() == 0) {
  *Length = 0;
  return nullptr;
}
StringRef S = GVEs[0]->getVariable()->getDirectory();
*Length = S.size();
return S.data();

1186

Similar with an early return.

1188

This is going to get "optimized" away in NDEBUG builds. If you hoist the S out of the cases, and sink the Length assignment and return, you at least get predictable behaviour.

1219

Similar behavioural changes as above.

1234

Similar behavioural changes.

1237–1240

Why not:

if (const auto *I = unwrap<Instruction>(Val))
  if (const auto &L = I->getDebugLoc())
    return L->getColumn();
return 0;

[hiraditya]

lib/IR/Core.cpp

1188

This assert will not trigger in a release build. Is there a better way to handle this?

1218

Same as above

[hiraditya]

lib/IR/Core.cpp

1170

getVariable can return nullptr

[jberdine]

Suggested changes to LLVMGetDebugLocDirectory and LLVMGetDebugLocColumn, will update LLVMGetDebugLocFilename and LLVMGetDebugLocLine when LLVMGetDebugLocDirectory settles

[jberdine]

lib/IR/Core.cpp

1162

Can further hoisting be done now?

1164

I added an assert above instead of _Nonnull since this is expected to be called from outside, where an attribute is less likely to be checkable.

1188

Do you expect the current version to be so "optimized"? (I'm not positive I grokked what you had in mind with 'sink the Length assignment and return'.)

[compnerd]

lib/IR/Core.cpp

1162

Not sure I understand the comment about _Nonnull not being able to check-able.

1169

The use of the goto is unnecessary. Just do:

if (GVEs.Size())
  if (const DIGlobalVariable *DGV = GVEs[0]->getVariable())
    S = DGV->getDirectory();

1176

The goto is unnecesary. Just do:

if (const DISubprogram *DSP = F->getSubprogram())
  S = DSP->getDirectory();

1188

Yes, it isn't unheard of for builds to be done without asserts enabled, so that is a valid concern.

1215

I think that this can be rewritten similar to the function above.

1230

I think that it makes sense to use the same pattern with the conditional assignments.

[hiraditya]

lib/IR/Core.cpp

1225

GVEs[0]->getVariable() can return nullptr.

[jberdine]

Rely on default initialization of StringRef, and use conditional assignment instead of early return.

lib/IR/Core.cpp

1162

This is an entry point that gets called from other language bindings via their foreign function interfaces, which are unlikely to check _Nonnull, or even be able to "see" it since they are essentially written against the ABI.

[compnerd]

lib/IR/Core.cpp

1162

The _Nonnull is a hint to the compiler that the value is non-null. When building that against a header that declares it, the compiler should ensure that the value being passed is non-null. The problem is that the assertion can be compiled out (-DNDEBUG). Maybe change this to a if (!Length) return nullptr?

1176

Add a return nullptr. The assertion can be compiled out.

1183

Same as above, either annotate with _Nonnull or change this to an early return.

1197

Same as above, add a return nullptr.

1204

Please change this to unsigned to match the return type of DebugLoc::getLine.

1217

Please add a return -1 here since the assert can be compiled out.

1223

Please change this to an unsigned to match the return type of DebugLoc::getColumn.

[jberdine]

early return instead of assert, unsigned instead of int

[compnerd]

This seems reasonable to me. @whitequark, are you okay with them too? To @whitequark's original point, I think that the dispatching the type based on a query is pretty cool (you can do an enumeration + union output), but, I think API-wise, this is safer and easier to work with, not to mention also simplifies the implementation.

[whitequark]

Looks good to me now, thanks @compnerd !

[jberdine]

Thanks for your help and patience! This has certainly improved a lot from the first version I submitted.

Would one of you mind committing this?

[jberdine]

rebase and squash

[compnerd]

SVN r344202

include/llvm-c/Core.h

  *
  * @see llvm::Module::getNamedMetadata()
  * @see llvm::MDNode::addOperand()
  */
 void LLVMAddNamedMetadataOperand(LLVMModuleRef M, const char *Name,
                                  LLVMValueRef Val);
 
 /**
+ * Return the directory of the debug location for this value, which must be
+ * an llvm::Instruction, llvm::GlobalVariable, or llvm::Function.
+ *
+ * @see llvm::Instruction::getDebugLoc()
+ * @see llvm::GlobalVariable::getDebugInfo()
+ * @see llvm::Function::getSubprogram()
+ */
+const char *LLVMGetDebugLocDirectory(LLVMValueRef Val, unsigned *Length);
+
+/**
+ * Return the filename of the debug location for this value, which must be
+ * an llvm::Instruction, llvm::GlobalVariable, or llvm::Function.
+ *
+ * @see llvm::Instruction::getDebugLoc()
+ * @see llvm::GlobalVariable::getDebugInfo()
+ * @see llvm::Function::getSubprogram()
+ */
+const char *LLVMGetDebugLocFilename(LLVMValueRef Val, unsigned *Length);
+
+/**
+ * Return the line number of the debug location for this value, which must be
+ * an llvm::Instruction, llvm::GlobalVariable, or llvm::Function.
+ *
+ * @see llvm::Instruction::getDebugLoc()
+ * @see llvm::GlobalVariable::getDebugInfo()
+ * @see llvm::Function::getSubprogram()
+ */
+int LLVMGetDebugLocLine(LLVMValueRef Val);
+
+/**
+ * Return the column number of the debug location for this value, which must be
+ * an llvm::Instruction.
+ *
+ * @see llvm::Instruction::getDebugLoc()
+ */
+int LLVMGetDebugLocColumn(LLVMValueRef Val);
+
+/**
  * Add a function to a module under a specified name.
  *
  * @see llvm::Function::Create()
  */
 LLVMValueRef LLVMAddFunction(LLVMModuleRef M, const char *Name,
                              LLVMTypeRef FunctionTy);
 
 /**

lib/IR/Core.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm-c/Core.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/GlobalAlias.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/IntrinsicInst.h"
void LLVMAddNamedMetadataOperand(LLVMModuleRef M, const char *Name,
   NamedMDNode *N = unwrap(M)->getOrInsertNamedMetadata(Name);
   if (!N)
     return;
   if (!Val)
     return;
   N->addOperand(extractMDNode(unwrap<MetadataAsValue>(Val)));
 }
 
+const char *LLVMGetDebugLocDirectory(LLVMValueRef Val, unsigned *Length) {
+  if (isa<Instruction>(unwrap(Val))) {
+    StringRef S = unwrap<Instruction>(Val)->getDebugLoc()->getDirectory();
+    *Length = S.size();
+    return S.data();
+  } else if (isa<GlobalVariable>(unwrap(Val))) {
+    SmallVector<DIGlobalVariableExpression *, 1> GVEs;
+    unwrap<GlobalVariable>(Val)->getDebugInfo(GVEs);
+    if (GVEs.size() != 0) {
+      StringRef S = GVEs[0]->getVariable()->getDirectory();
+      *Length = S.size();
+      return S.data();
+    } else {
+      *Length = 0;
+      return nullptr;
+    }
+  } else if (isa<Function>(unwrap(Val))) {
+    const DISubprogram *DI = unwrap<Function>(Val)->getSubprogram();
+    if (DI) {
+      StringRef S = DI->getDirectory();
+      *Length = S.size();
+      return S.data();
+    } else {
+      *Length = 0;
+      return nullptr;
+    }
+  }
+  assert(0 && "Expected Instruction, GlobalVariable or Function");
+}
+
+const char *LLVMGetDebugLocFilename(LLVMValueRef Val, unsigned *Length) {
+  if (isa<Instruction>(unwrap(Val))) {
+    StringRef S = unwrap<Instruction>(Val)->getDebugLoc()->getFilename();
+    *Length = S.size();
+    return S.data();
+  } else if (isa<GlobalVariable>(unwrap(Val))) {
+    SmallVector<DIGlobalVariableExpression *, 1> GVEs;
+    unwrap<GlobalVariable>(Val)->getDebugInfo(GVEs);
+    if (GVEs.size() != 0) {
+      StringRef S = GVEs[0]->getVariable()->getFilename();
+      *Length = S.size();
+      return S.data();
+    } else {
+      *Length = 0;
+      return nullptr;
+    }
+  } else if (isa<Function>(unwrap(Val))) {
+    const DISubprogram *DI = unwrap<Function>(Val)->getSubprogram();
+    if (DI) {
+      StringRef S = DI->getFilename();
+      *Length = S.size();
+      return S.data();
+    } else {
+      *Length = 0;
+      return nullptr;
+    }
+  }
+  assert(0 && "Expected Instruction, GlobalVariable or Function");
+}
+
+int LLVMGetDebugLocLine(LLVMValueRef Val) {
+  if (isa<Instruction>(unwrap(Val))) {
+    const DebugLoc &L = unwrap<Instruction>(Val)->getDebugLoc();
+    return (L ? L->getLine() : 0);
+  } else if (isa<GlobalVariable>(unwrap(Val))) {
+    SmallVector<DIGlobalVariableExpression *, 1> GVEs;
+    unwrap<GlobalVariable>(Val)->getDebugInfo(GVEs);
+    return (GVEs.size() > 0 ? GVEs[0]->getVariable()->getLine() : 0);
+  } else if (isa<Function>(unwrap(Val))) {
+    const DISubprogram *DI = unwrap<Function>(Val)->getSubprogram();
+    return (DI ? DI->getLine() : 0);
+  }
+  assert(0 && "Expected Instruction, GlobalVariable or Function");
+}
+
+int LLVMGetDebugLocColumn(LLVMValueRef Val) {
+  Instruction *I = unwrap<Instruction>(Val);
+  assert(I && "Expected Instruction");
+  const DebugLoc &L = I->getDebugLoc();
+  return (L ? L->getColumn() : 0);
+}
+
 /*--.. Operations on scalar constants ......................................--*/
 
 LLVMValueRef LLVMConstInt(LLVMTypeRef IntTy, unsigned long long N,
                           LLVMBool SignExtend) {
   return wrap(ConstantInt::get(unwrap<IntegerType>(IntTy), N, SignExtend != 0));
 }
 
 LLVMValueRef LLVMConstIntOfArbitraryPrecision(LLVMTypeRef IntTy,