Thank you, I'll submit this shortly.

Sorry, I shouldn't really have said *suggest*. Can you please rename GetSubtypes to Subtypes? GetFoo is not idiomatic in Go (there are some instances of GetFoo in the existing code, but they're stuck like that until someone decides to break compatibility).

Code looks fine, and thanks for adding a test. I just think this one should be a method rather than a free function.

Thanks.

Actually, I've just run the tests and it's asserting:

LGTM

In D28215#710769, @ararslan wrote:

Thanks for taking a look at this, @axw. Is there anything else that needs to happen before this can be merged?

In D30042#679601, @rhysd wrote:

Thank you for your review and approve. I don't have a permission to commit. Could you please commit this patch to repository?

LGTM, thanks.

Sorry for the delay.

LGTM, sorry for the delay.

Couple of little things, LGTM otherwise.

LGTM

LGTM

@pcc ping?

@pcc ping?

@pcc PTAL. Sorry for leaving this for so long.

Addressed review comments

• removed source printing flag
• moved __llgoiResults extraction to interpretLine
• replaced existing CHECKs in lit tests

Ping.

In D18355#381860, @cierniak wrote:

In D18355#380939, @axw wrote:

LGTM, thanks. One of these days we should expose llvm::DEBUG_METADATA_VERSION to the C API.

I don't have commit access to LLVM yet (this is my first patch ever). Could you commit it for me?

LGTM, thanks. One of these days we should expose llvm::DEBUG_METADATA_VERSION to the C API.

@pcc the newer gofrontend revision has the necessary fixes. check-libgo no longer hangs, and prints out results as they're available. OK to land now?

Closed by rL263536

In D18173#375236, @pcc wrote:

Maybe this should be renamed CreateTargetData to emphasize that the TargetData is owned by the caller. We probably need to update the code in llgo for the new ownership as well.

TargetMachine.TargetData -> TargetMachine.CreateTargetData

Rename TargetMachine.TargetData to TargetMachine.CreateTargetData

Ping. Once this is in, I'll look at updating to 1.6.

IMO this is good to go, but I'm biased as I was involved in writing it. I'll resign as reviewer for that reason. @beanz, PTAL.

@pcc Thanks, PTAL.

• Fix autoconf file copying
• Call hash/equal functions directly, not through descriptors.

I hope what I've said is clear - please ask if not. I can see if I can find some time to propose an alternative, but also very busy with day job.

Superceded by D15188

@pcc Ping. This is ready for review now.

In D15188#331740, @axw wrote:

Revert removal of __go_runtime_error

No longer causing errors. Have not bisected
to find out what the issue is yet.

Superceded by D16392

In D16387#331920, @pcc wrote:

For both libLLVMGlobalISel and libLLVMDebugInfoCodeView? The former would be lacking a build rule due it being optional, but was causing llvm-config to complain due to the component not being marked optional. I've fixed that in r258379.

Okay, I see.

I don't think that helps. As far as I can see, that list of components is only used by the standalone Go bindings build (bindings/go/build.sh).

It used to be passed to llvm-config by llvm-go.go until your change r258283, and things worked until then. As you are fixing build breakage I would have preferred a more minimal change that reverts things to the way they used to work. But this is fine as well, LGTM.

In D16387#331898, @pcc wrote:

Does this solve the problem?

Remove unrelated changes

Revert removal of __go_runtime_error

In D16208#328470, @axw wrote:

In D16208#327770, @delcypher wrote:

Looks like this got merged before I was able to comment on this.

Sorry, I probably should have left it open a bit longer. Your suggestions read well, I'll commit them early next week.

In D16208#327770, @delcypher wrote:

Looks like this got merged before I was able to comment on this.

I have reverted this, because it breaks if you have BUILD_SHARED_LIBS=on and we're about to branch for 3.8. I'll reapply when D15986 (or another fix) is ready to land.

I've just come back to this and found that it's broken. Unsure of what's changed in the interim, but I get "panic: panic during panic" caused by type descriptor comparisons. Will come back to this when I have time.

Revert comment change