Jonas' comments

updated.

OK, on it.

Add an unittest maybe? otherwise, seems to match the ISA so LGTM

I'm not necessarily sure I like this. It's simply spreading in a general framework a concept that shouldn't exist to begin with. I'd rather remove forced constant from SCCP, even though it might result in us losing some optimization power.

In D60300#1456472, @clayborg wrote:

In D60300#1456468, @JDevlieghere wrote:

In D60300#1455790, @davide wrote:

I think this is good regardless for readability, but I would really appreciate if we can collect some numbers to see how effective this change actually is.

I see 120 -> 12 seconds on my iMac Pro.

Nice! Might be nice to have some automated info that gets dumped when one file with multiple tests takes a long time and suggest breaking them up in the python test infrastructure modules. Might be other nice gains to be had?

In D60300#1456491, @davide wrote:

In D60300#1456472, @clayborg wrote:

In D60300#1456468, @JDevlieghere wrote:

In D60300#1455790, @davide wrote:

I think this is good regardless for readability, but I would really appreciate if we can collect some numbers to see how effective this change actually is.

I see 120 -> 12 seconds on my iMac Pro.

Nice! Might be nice to have some automated info that gets dumped when one file with multiple tests takes a long time and suggest breaking them up in the python test infrastructure modules. Might be other nice gains to be had?

lit has this feature. It's called --time-tests. That's how we discovered the problem. There are other tests that we can fix.

--Davide

great!

I think this is good regardless for readability, but I would really appreciate if we can collect some numbers to see how effective this change actually is.

Thanks.

LGTM. Med, what do you think?

LGTM

LGTM

In D59911#1446787, @aprantl wrote:

Thanks for summarizing your thoughts, Davide.

I think that what you wrote is a much better explanation of what I was trying to say with

Use these sparingly and only if error handling is not otherwise feasible.

I think that unless we can eliminate all uses of lldb_assert() from the code we should document it, but discourage its use. Do you have any suggestions for a better wording?

My (maybe unpopolar) opinion on the subject is that "soft assertions" are a way to cleanse your conscience of guilt, but they don't work really well in practice.
When I started working on lldb, I was a fairly strong proponent of assertions everywhere. My view changed somewhat radically over the course of the past 18 months, and I would like to summarize some points here.

This looks correct to me, but I'm not extremely familiar either, so I'd wait for @labath to sign off.

In D59896#1445310, @jasonmolenda wrote:

Looks good. Removing that printf is good. Could you also remove the printf("mach_header:...") in the same function?

It would be nice if include/lldb/Target/Process.h had the decl for ReadMemoryFromInferior right after ReadMemory(), and a comment that ReadMemoryFromInferior does not go through the memory cache (and most callers should go through ReadMemory()) so people know what the difference is.

try

LGTM

In D59719#1441254, @labath wrote:

It sounds to me like you could achieve the same thing by generalizing the LLDB_PYTHON_HOME logic in LLDBConfig.cmake. This would have the advantage of centralizing the way we manage python-finding logic (instead of each OS doing it's own thing) and also enable those users, who know what they are doing, to override this logic and point lldb to a different python. (I don't know if there are any such users, but it does not sounds like an impossible scenario).

I think all it would take is to do something like:

• move LLDB_RELOCATABLE_PYTHON handling outside of if(WINDOWS)
• have the default value of LLDB_RELOCATABLE_PYTHON be false for darwin
• possibly tweak the python-finding logic so that it prefers the one in /System/Library/Frameworks/...

In D59719#1441254, @labath wrote:

It sounds to me like you could achieve the same thing by generalizing the LLDB_PYTHON_HOME logic in LLDBConfig.cmake. This would have the advantage of centralizing the way we manage python-finding logic (instead of each OS doing it's own thing) and also enable those users, who know what they are doing, to override this logic and point lldb to a different python. (I don't know if there are any such users, but it does not sounds like an impossible scenario).

I think all it would take is to do something like:

• move LLDB_RELOCATABLE_PYTHON handling outside of if(WINDOWS)
• have the default value of LLDB_RELOCATABLE_PYTHON be false for darwin
• possibly tweak the python-finding logic so that it prefers the one in /System/Library/Frameworks/...

In D59719#1442181, @ted wrote:

This doesn't look correct to me - it looks like there are 1 too many #endifs. I think the one at line 179 should be removed - it should have been replaced by the #else that is at line 180.

LGTM.

Adrian's feedback

LGTM, just clang format main.cpp before committing.

This looks good to me as long as it doesn't break check-lldb (and from I quick look at the patch I doubt it does). Thanks for helping with this!

Thanks for your interest in the subject of Python! I converted everything I was able to find looking at our test suite, it seems that we lack coverage for these scripts.
Did you end up testing them with both py 2 and py 3 or just applied a tool on them?