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

[Jakob Bornecrantz via llvm-commits]

Wallbraker accepted this revision.

================
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);
+
----------------
whitequark wrote:
> 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.
Nitpicking: LLVMConstString* functions does include a Length argument, as does the new functions in D18727.

I have to agree it would be inconsistent, I will retract my comment and refer the issue to other stake holders in the C API.


http://reviews.llvm.org/D18736