Well, I'm fine with x-failing it on Linux, even though I guess at some point someone (i.e., probably me) has to figure out why this stuff is broken in the expression parser.

Just dumping this here because I don't want to anger the build bots before going into the weekend.

Sorry, seems like I forgot to delete these tests when I extracted them into their own test function.

Yeah, seems like constructing objects in expressions isn't implemented on Windows. I'm not sure if there is a reliable way to test that constructors aren't shadowed by these global functions if constructors themselves don't work on Windows, but I filed llvm.org/pr43707 for the underlying bug and x-failed the tests. Will push in a few minutes.

LGTM, thanks!

+1

I filed a bug for you, please reference it and submit: https://bugs.llvm.org/show_bug.cgi?id=43702 (we can update the bug description later).

(Seems like my previous comment was cut off in the middle for some reason?)

Oh, I wasn't aware we already had that in the allocator. I looked at the source and it seems that we need to first create about 1024 slabs before we reach the 1MiB limit, which means we need 256 * 1024 = 262144 slabs in LLDB due to the 256 different string pools.

This patch actually made the RSS memory in our benchmarks go up by quite a bit (see lldb-bench ). The reason is that while we only create 1MiB chunks in the new allocator, we do have 256 different string pools we use as the backend for ConstString. So this means that we actually do allocation steps of 1MiB * 256 = 256MiB which is quite a lot. On a side note, we do use ConstString in the lldb-debugserver, and allocating additional 256MiB on some device running lldb-debugserver could end up being a serious problem if my understanding is correct? (+Alex because IIRC he cares about the debug server memory footprint).

So, are there any concerns about this (beside the typo which I'll fix pre-commit) and Shafik's request for documentation (which will be another NFC commit)?

(Adding Jonas because reproducers)

Thanks for the explanation! Doesn't seem like removing this breaks Swift, so I'll land this.

LGTM, thanks a lot!

• GetHasSourceSize -> HasSourceSize
• Moved some code around according to feedback.

I just realised the same is true for addr (beside that it is consistently named). I'll remove this too after this has landed.

Btw, the current declaration of MemorySource is just what I came up with on the fly. Not sure where the documentation should go or if that class deserves its own header.

LGTM. Also, this whole function seems to have no tests. So if you find an easy way to test this, then please add a test to functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py. You can also check this in as-is if the test turns out to be too complicated (I'm not a big fan of the whole "you touched my spaghetti so you also have to come up with a way to test the whole thing".)

Also LGTM! Thanks!

• Addressed feedback (Thanks Gabor, Adrian & Shafik!)

Just a few nitpicks about some minor typos, otherwise this LGTM. Thanks for the patch! I assume you need someone to commit this for you?

A few short comments what the role of the different classes/members play in the test case would be helpful. E.g. "This member/variable/expressions triggers the loading of Decl Foo in that CU". I'll take a closer look tomorrow, but on the first looks this patch LGTM. Thanks!