[llvm-commits] [PATCH 5/5] [llvm-c] Add LLVMMessageRef for user-owned strings.

[Anders Waldenborg]

On Wed, Mar 21, 2012 at 09:12:35PM -0700, Gregory Szorc wrote:
> > 
> > Adds a "typedef char *LLVMMessageRef;" to llvm-c.
> 
> I'm not in favor of this. If the type provided some additional
> functionality over merely wrapping char *, I might be swayed.

IMHO it does add additional functionality: it adds a single point that
documentation can be added to and code generators can trigger on.

For the code generation, which was my main motivation for this, there
are other ways. But for documentation it would provide an obvious
place to put the documentation on in just one place, that
automatically will be linked by doxygen from all uses.

But if people feels it is a stupid idea I'll just drop it.

> C programmers just know what to do with char *. By introducing a wrapper
> type, you undermine that accumulated knowledge. All of a sudden people
> are like "can I use it in strcmp?," etc.

That would be the downside. But I think learning that it is just
another word for "(NUL terminated) char *" is something that one
learns fast.

It is somewhat similar to the LLVMBool typedef.

> I believe the problem that needs addressed is documentation. Currently,
> not many functions tell you what's going on with memory. Any function
> allocating memory, returning a pointer, etc needs to explicitly call out
> who owns what memory and (if applicable) the lifetime of that memory. If
> the caller is responsible for the memory, the docs need to state the
> proper way to dispose of that memory.

I find this to be a much larger issue, which I totally agree with.

> In this case, since we're dealing with char * in a C API, I think the
> use of free() is implied.

Except that in this case LLVMDisposeMessage should be used :)

> Also, with char *, we need to document whether
> the allocated buffer is NULL terminated.

> If a function creates a char *,
> it should ideally also take a size_t * which is filled with the length
> of the new buffer.

Yes, that would be nice. One alternative would be to make the
LLVMMessage a non-opaque struct containing char * and size_t.

 anders