Fix compiling LLDB

[tambry]

Rebased onto latest main and a compile fix for LLDB to get this up internally for testing.
Some minor merge conflicts due to 9434c08, but those were easily resolved.

Hopefully this doesn't make too much of a mess! 😅
If you can, please run the tests yourself. I seem to have some environment issues causing many unrelated failures, which I'm looking into.

[katzdm]

Hey @tambry , thanks for the PR! Just to check - is this a pure rebase from upstream LLVM? I've been merging (rather than rebasing) upstream into this branch at a roughly weekly cadence. For the time being, I'd prefer to drive the merges with upstream - only because PRs like this one (i.e., 261 commits, 880 files affected) are somewhat infeasible to review.

That said, it sounds like you might also have a compile-fix for LLDB within this PR? Let me merge from upstream now; once I push that, I'd love to look at your fix in a separate and self-contained PR.

If you can, please run the tests yourself.

I've been using ninja check-all to validate most changes before pushing - let me know if there's a broader suite of tests that would catch a failure missed by that suite. And sorry for the current lack of CI! We'll try to get that moving soon, but we wanted to get this implementation out before the WG21 meeting in Tokyo last month.

Thanks again for contributing!

[katzdm]

Fyi I've just merged latest for upstream.

[tambry]

Hey @tambry , thanks for the PR! Just to check - is this a pure rebase from upstream LLVM?

Correct. Plus a small LLDB compile fix.

I've been merging (rather than rebasing) upstream into this branch at a roughly weekly cadence. For the time being, I'd prefer to drive the merges with upstream - only because PRs like this one (i.e., 261 commits, 880 files affected) are somewhat infeasible to review.

Understandable. Prior to discovering this I had just yesterday upgraded my company's LLVM. I had some follow-up fixes I needed to do a re-build for, so I took the time to also use this as a base instead, but as such had to rebase it to be at least equivalent. Hopefully I'll keep tracking this so shouldn't be an issue going forward. 🙂

That said, it sounds like you might also have a compile-fix for LLDB within this PR? Let me merge from upstream now; once I push that, I'd love to look at your fix in a separate and self-contained PR.

Rebased, should be sensibly reviewable now.

I've been using ninja check-all to validate most changes before pushing - let me know if there's a broader suite of tests that would catch a failure missed by that suite.

Since I'm building this to distribute within my company we have quite a few downstream patches, which seem to be interfering with the test suite. As such I had 0 confidence in my test runs actually being of any use as I was seeing hundreds of failures and the LLDB suite didn't run at all. I plan to look into these and hopefully fix them soon, but it didn't seem worth waiting for that. 😊

Thanks again for contributing!

My aim is to deploy this internally and give it a whirl within our applications. Maybe help a tad too, if I can find the time.

[katzdm]

Ah, I see! I hadn't been building and testing lldb - I'll be sure to start doing so.

My aim is to deploy this internally and give it a whirl within our applications.

I'm psyched to hear that you'll be trying out our project! I do, however, want to emphasize the following warning from our project summary document.

The Clang/P2996 fork is highly experimental; sharp edges abound and occasional crashes should be expected. Memory usage has not been optimized and is, in many cases, wasteful. DO NOT use this project to build any artifacts destined for production.

I can't stress this enough: There are known issues - crashes, poor compiler memory usage, etc - with this project, and everything that it implements will continue to change fast. We offer no guarantees that code working this week will continue to work next week: The P2996 proposal is continuing to evolve, and this implementation will evolve with it. It is not suitable for more than experimentation, and I strongly advise against deploying any artifact produced from it at this time.

Maybe help a tad too, if I can find the time.

With all of that said - I'm now seeing the build failure in lldb, and I'm excited to take a look at your patch 😄

[katzdm]

Ah - I think this might actually not be quite right. The second-to-last argument (i.e., GetAsCXXRecordDecl(type)) needs to be the CXXRecordDecl of the derived class - as far as I can tell, type refers to the base class.

Some context: In order to implement the is_accessible metafunction for base classes, it became necessary to have an "edge" from the CXXBaseSpecifier (which we internally use to represent a "reflection of a base class") back to the derived class. It's otherwise not possible to reason about e.g., whether a private base class is visible from a friend of the derived class.

[katzdm]

Definitely in the right direction, but I think a little more work is needed to pipe the declaration of the derived class to the CXXBaseSpecifier.

[tambry]

Oops, pushed my local rebase. Please ignore. 😬
I really should've made this PR off a separate branch dedicated for this instead of my local "main".

I'm still working on getting the LLVM test suites running with my downstream patches.
Having to pay some technical debt accumulated over the years for this. 😅

[katzdm]

Hey @tambry ! Just wanted to check if you're still interested in pursuing this fix - if not, I can take a crack at it based on your existing work 🙂

[tambry]

@katzdm I still have interest but I'm on vacation for the next 2 weeks.
It's probably more efficient if you can have a crack at it. 😊

[katzdm]

@katzdm I still have interest but I'm on vacation for the next 2 weeks. It's probably more efficient if you can have a crack at it. 😊

Thanks for the heads up, and enjoy the time away! I'll take a look if I can find the time :)