[PATCH] D18736: [llvm-c] Improve IR Introspection: Add LLVM{Get, Set}ModuleIdentifier

[whitequark via llvm-commits]

whitequark added inline comments.

================
Comment at: include/llvm-c/Core.h:483-496
@@ -482,2 +482,16 @@
 
 /**
+ * Obtain the identifier of a module. The result must be discarded with
+ * LLVMDisposeMessage.
+ *
+ * @see Module::getModuleIdentifier()
+ */
+char *LLVMGetModuleIdentifier(LLVMModuleRef M);
+
+/**
+ * Set the identifier of a module.
+ *
+ * @see Module::setModuleIdentifier()
+ */
+void LLVMSetModuleIdentifier(LLVMModuleRef M, const char *ID);
+
----------------
Wallbraker wrote:
> Please add a length parameter for both functions. Not all languages keep their string zero terminated and keep track of the length. For C StringRef will call strlen so the cost is the same.
> 
> ```
> char *LLVMGetModuleIdentifier(LLVMModuleRef M, size_t *Length);
> void LLVMSetModuleIdentifier(LLVMModuleRef M, const char *ID, size_t Length);
> ```
No other function in llvm-c currently does this. I don't see how the nonexistent benefit of being able to add a non-zero-terminated string for a human-readable identifier overweighs the drawback of making the API inconsistent.


http://reviews.llvm.org/D18736