- User Since
- Sep 23 2014, 5:24 PM (251 w, 5 d)
Thu, Jul 18
This looks fine to me. Makes it really clear that we need SBTarget::FindFirstTypeForLanguage, etc. But FindFirstType was always a crapshoot anyway...
Wed, Jul 17
This looks good in general. I also have a few nits, and you need to fix (and probably test) the behavior for files with spaces in their names.
I wonder if we ought to allow target properties (target as an example) that are only for testing, so they don't print when you do settings list, etc. But the we could have some settings like a "target.testing.dont-read-LC_MAIN" and that would make it easy to automate your hand testing. Kind of like the "experimental" decorator, except you have to know they exist to use them...
The nice thing about the way the ObjCLanguageRuntime::Get was that is was only useable where we decided it was legit to use it, in the actual ObjCLanguageRuntime plugin or its direct users. If you want to keep the convenience cast function in a header in Plugins/ExpressionParser/Clang, that would be fine.
Seems like in most places we're just pulling out basic types or their sizes, which we should certainly be able to do with a TypeSystem.
I think I understand your goal - a worthy one, BTW... But I think this is an unnecessary step along that path.
This seems partly wrong to me. The point is that the Target holds keeps scratch AST contexts for all the languages it supports. They are the central repository for the accumulation of effects of expressions evaluated while that target is alive. For instance, all the user defined types and variables go there. The Target also manages its lifecycle. So all the Scratch AST Contexts are properly owned by the target. For instance, in the swift case if there's a module that we can't import, we have to fall back to making an AST Context for each module we can successfully import and dispatching expressions to the appropriate one of those.
Fri, Jul 12
That looks good.
Thanks for writing this up!
Thu, Jul 11
That would be cleaner.
Then the IRDynamicCheck part would go with LLVMUserExpression, and the C-specific checks in Clang...
Okay... Provided we can come up with not too torturous ways to get the ObjC and Swift support out of generic-code, it seems okay to do this as a first step. I just don't want to end up in the state where we have ObjCLanguageRuntime as generic but CPPLanguageRuntime is not.
This is a little unclear to me. LLVMUserExpression is the class that represents "anything that uses LLVM at its back end." Both the Swift & Clang user expressions are instances of this. Running through IR and inserting little callouts is an LLVMUserExpression type thing. So it seems like this move is in the right direction, but to be really clean needs to represent LLVMUserExpression in the Plugin Hierarchy.
My model for this was that there was a CPPLanguageRuntime.cpp that contains everything you can implement about the CPP runtime that is independent of any particular implementation of the CPP language runtime, and then a plugin instance (in this case the ItaniumABILanguageRuntime) that contains all the bits that are specific to a particular implementation. Then putting the CPPLanguageRuntime.cpp in Target lets you know that this has to only contain the generic parts of the runtime behavior. That still seems to me a useful distinction.
Mon, Jul 8
Do you know how you are going to do enum option values?
Why did you make this a function of Variable, rather than SymbolContextScope?
Wed, Jul 3
Tue, Jul 2
Mon, Jul 1
Fri, Jun 28
Addressed Greg's comments
This looks good to me. I wonder if the SymbolContextScope -> Language calculation that you do in IsRuntimeSupportValue should really be done in SymbolContextScope? If that's going to be the policy for going from SymbolContextScope, maybe centralize it there?
Thu, Jun 27
Addresses Greg's question about what happens when we load a new OS plugin. Indeed we should limit the work we do only to the case where we didn't have an OS plugin, then tried to load one and succeeded. Only then do we need to clear and update the thread list.
Wed, Jun 26
Shouldn't ValueObjectVariables figure out their runtime language from their defining frame, not their CompilerType? For a ValueObject you get from an expression, you probably can't do that. But we're always talking about hiding locals or args here - i.e. they are all ValueObjectVariables. And it seems to me that in that case getting the RuntimeLanguage from the containing frame is much more useful than from the CompilerType.
Mon, Jun 24
Jun 21 2019
Jun 20 2019
This change makes it clear that SBTarget::FindFirstType should take a language, but that is orthogonal to this change.
Jun 18 2019
This looks fine to me.
Jun 4 2019
Pretty much the only actual effect of this patch (besides its changes to the dependency graph) are introducing the possibility for ambiguity here. It seems more logical to do it all as one patch.
Jun 3 2019
This looks fine to me.
I have no problem with the change in general. However, you've introduced the possibility of name collision between convenience type definition in various languages. So it would be good to run through the persistent DECL's for ALL the supported languages and report collisions. Probably also need to add a -l flag to pick a language?
Fine by me.
This deals with my objections so I'm marking as accepted because that's the only way I can see to unblock. Seems like you were still discussing return types, and I don't want to preempt that, however...
May 31 2019
This seems like carrying purity a little too far. You haven't removed the fact that the using code is explicitly dialing up the C++ language runtime, you've just made the call-site a little harder to read.
May 30 2019
There should be a test to go along with this.
May 29 2019
The typical trick for doing argument substitution before debugging was roughly to debug:
May 22 2019
May 21 2019
Then put in a comment saying something like "LLDB ONLY stores history files in .lldb and if you don't like that..." If you are instituting a policy which is not common you should at least document it...
Most other programs write their history files in ~. So we are being a little odd in offering to put them in ~/.lldb, though I agree that is convenient.
I don't think that's quite right. I think the behavior should be:
Why are we calling this "widehistory" because we couldn't write into the .lldb directory? I know that's the way it was but it makes no sense. I guess it would make sense to call it widehistory if edit line was in wchar mode, that way you wouldn't ask a non wchar edit line to read a wchar history file...
May 15 2019
May 14 2019
Getting it from the Process's m_language_runtimes is probably fine. On reflection, I can't think of a reason why you would want to iterate over all the available LanguageRuntimes, including the ones that hadn't been recognized in this Process yet.
There is a TypeSystemEnumerateSupportedLanguages that we use so that we don't have to enumerate over all the language in the languages enums. After all the plugin manager knows which languages we have type systems for. If you're going to be doing a lot of this generalization, can you add a similar enumeration for the LanguageRuntimes? There are 40 or so languages in the language enum but we only have a couple of LanguageRuntimes...
May 13 2019
Excellent, thanks! Can you check this in or do you need me to?
May 10 2019
With appropriate comments, this seems fine.
I really don't want to pollute the lldb driver options with gdb equivalents (or really any duplicate spellings of already present functionality). For the lldb command line, I want us to have the freedom to do the best job of making the lldb options consistent and easy to document, understand and use, and adding in random duplicate options from gdb will only make this harder.
If you really are going to support many languages you need to figure out how to tell folks what really happened with more specificity. For instance, you can use C++ to throw anything, so you could throw an ObjC object from a C++ exception throw. So you need to distinguish between the language of the exception thrown (which is captured by the ValueObject you return) and the language runtime throwing the language. So we need a way to query that. Also, there's no formula reason why you couldn't have two languages throwing an exception at the same time (for instance if a C++ exception is unwinding the stack and the destructor of some ObjC object that is getting destroyed throws an NSException, etc... So there needs to be some way to handle that.
May 9 2019
The one outstanding bit of work here is that this change requires that the MSInst "IsCall" function has to mean "will return to the next instruction after call" or we might lose control of the program. It seems obvious that that SHOULD be what it means, but we need to make sure that is always going to be what it means or we risk losing control of the program. Greg is going to follow up on that.
I would rather not clutter up the lldb command driver's options with gdb command flags. That seems like it will make lldb harder to figure out and reduce our freedom to choose reasonable short names for lldb driver options.