LGTM after that one small go style change.

Closed by revision rL328953.

@whitequark Do the Go bindings changes look good to you? If so, I'll commit this.

@echristo Do these Go bindings changes look good?

Fix Go bindings

• Rebase on master

Before I commit this: @echristo this is also going to obviate some of the Go bindings and require changes...

• Update comments to /** style
• Fix c-style doc comment

• Fix remaining comment issues

Clean up DIBuilder.h comments

This needs more comprehensive testing but I wanted to get an initial code review pass over this. Thanks y'all!

Changes rebased on master.

wq - Fix 80-column limit for LLVMDWARFSourceLanguage

• Address comments
• uint64_t -> size_t for string lengths
• Add simple debuginfo tests
• Fix unused variable warning
• uint8_t -> LLVMBool for flags
• ProducerLen should be size_t
• Add test and update CMakeLists to build debuginfo.c
• Fix test
• Use unsigned instead of explicitly sized unsigned integer types
• Add comment explaining 0 for debug locations
• Expand enum definitions and translate to DI (#1)

I don't have commit access -- would you mind merging it, @whitequark?

@whitequark Think this is good to merge in its current state? I have another patch for more functions Moreno or less ready to go once this is in.

• Added explicit enum definitions for C API DWARF values (Change made by @CodaFi)

• Add comment explaining 0 for debug locations

• Fix 80-column limit for LLVMDWARFSourceLanguage
• Address comments
• uint64_t -> size_t for string lengths
• Add simple debuginfo tests
• Fix unused variable warning
• uint8_t -> LLVMBool for flags
• ProducerLen should be size_t
• Add test and update CMakeLists to build debuginfo.c
• Fix test
• Use unsigned instead of explicitly sized unsigned integer types

• Fix 80-column limit for LLVMDWARFSourceLanguage
• Address comments
• uint64_t -> size_t for string lengths
• Add simple debuginfo tests
• Fix unused variable warning
• uint8_t -> LLVMBool for flags
• ProducerLen should be size_t
• Add test and update CMakeLists to build debuginfo.c

• uint8_t -> LLVMBool for flags
• ProducerLen should be size_t

• Fix unused variable warning

• Add simple debuginfo tests

• uint64_t -> size_t for string lengths

• Fix 80-column limit for LLVMDWARFSourceLanguage
• Address comments

• Change noreturn interface to expose NoReturn as an attribute

• Fix 80-column limit for LLVMDWARFSourceLanguage
• Pass string parameters as pointer/length pairs

Add back remanining files (sorry for the email spam!)

Fix 80-column limit for LLVMDWARFSourceLanguage

Fix merge head

arc...picked some wrong branches here. I'll fix this diff 😅

This fails to link when I compile it locally, and I have absolutely no idea why. So this is still untested.

Add test for clang_Cursor_isFunctionNoReturn

I do not have commit rights. If you could merge, that'd be great. Then I'll submit the first subset of DIBuilder APIs!

• Fix typo

@echristo, @whitequark, @aprantl Are we good to merge this?

Simplifies the patch to just define LLVMDIBuilderRef

I seem to have messed up this patch.

I talked with @CodaFi about this API; he's been modeling the wrapper for LLVMSwift based on this initial review just to get a feel for the API.

In D32122#728263, @deadalnix wrote:

There are a lot of binding out there that we just aren't aware of. And getting failure at runtime because the expected arguments to a function do not match anymore is seriously not funny to debug.

Update the definitions for LLVMDIBuilderRef and LLVMMetadataRef to be more in line with how the rest of the opaque types are declared.

In D32122#728233, @echristo wrote:

That said, would random changes to this API be upsetting Harlan? :)

I don't think that's correct.

Sorry, I should've provided more details. I ran llvm-cov with -output-dir swift-coverage, which output everything, including the index file, into swift-coverage.

I just ran this on an instrumented Swift code run, and it worked really great! One issue, though; the coverage was output to /home/harlan/swift-coverage, but the hyperlinks pointed to swift-coverage/..., meaning that all the hyperlinks were relative to the parent of the index file.

Looking at the bitcode compatibility coverage example, it looks like those hyperlinks in the index page are absolute links to /tmp/llvm-as.dir/coverage/<file>

Justin's recommendations amount to a refactor of much of llvm-cov, and unfortunately I don't have the time right now to overhaul this change. I'll try to get to it soon, but things have ramped up here.

In D18278#396366, @davidxl wrote:

Any update on the patch? Justin had some made some comments ..

Removed colon from HTML function title

Removed binary profdata files and updated tests