> On Jul 21, 2020, at 9:27 AM, Pavel Labath <pa...@labath.sk> wrote: > > On 20/07/2020 23:25, Jim Ingham wrote: >> It seems like you are having to work hard in the ValueObject system because >> you don’t want to use single AST Type for the ValueObject’s type. Seems >> like it be much simpler if you could cons up a complete type in the >> ScratchASTContext, and then use the underlying TypeSystem to do the layout >> computation. >> >> Preserving the full type in the scratch context also avoids other problems. >> For instance, suppose module A has a class that has an opaque reference to a >> type B. There is a full definition of B in modules B and C. If you make up >> a ValueObject for an object of type A resolving the full type to the one in >> Module B you can get into trouble. Suppose the next user step is over the >> dlclose of module B. When the local variable goes to see if it has changed, >> it will stumble across a type reference to a module that’s no longer present >> in the program. And if somebody calls RemoveOrphanedModules it won’t even >> be in the shared module cache. >> >> You can try to finesse this by saying you can choose the type from the >> defining module so it can’t go away. But a) I don’t think you can know that >> for non-virtual classes in C++ and I don’t think you guarantee you can know >> how to do that for any given language. >> >> I wonder if it wouldn’t be a better approach to build up a full >> compiler-type by importing the types you find into the scratch AST context. >> That way you know they can’t go away. And since you still have a full >> CompilerType for the variable, you can let the languages tell you where to >> find children based on their knowledge of the types. >> > > I do see the attractiveness of constructing of a full compiler type. The > reason I am hesitant to go that way, because it seems to me that this > would negate the two main benefits of the frame variable command over > the expression evaluator: a) it's fast; b) it's less likely to crash. > > And while I don't think it will be as slow or as crashy as the > expression evaluator, the usage of the ast importer will force a lot > more types to be parsed than are strictly needed for this functionality. > And the insertion of all potentially conflicting types from different > modules into a single ast context is also somewhat worrying.
I agree here. Frame variable should do as little as possible when dealing with a ValueObject and its type, so only completing the parts of the type we know are transparent it a good approach. > > The dlclose issue is an interesting one. Presumably, we could ensure > that the module does not go away by storing a module shared (or weak?) > pointer somewhere inside the value object. BTW, how does this work with > ValueObject casts right now? If I cast a ValueObject to a CompilerType > belonging to a different module, does anything ensure this module does > not go away? Or when dereferencing a pointer to an type which is not > complete in the current module? I am not sure dlclose is a problem, the module won't usually be cleaned up. And that shared library shouldn't have the definition we need and be able to be unloaded IIUC how the -flimit-debug-info stuff works. > > I'm hoping that this stuff won't be "hard work". I haven't prototyped > the code yet, but I am hoping to keep this lookup code in under 200 LOC. > And as Greg points out, there are ways to put this stuff into the type > system -- I'm just not sure whether that is needed given that the > ValueObject class is the only user of the GetIndexOfChildMemberWithName > interface. The whole function is pretty clearly designed with > ValueObject::GetChildMemberWithName in mind. We should be able to code it into ValueObject, or maybe just into TypeSystem base class? > > Another thing I like about this approach is that it will mostly use the > same code path for the limit and no-limit debug info scenarios. OTOH, > I'm pretty sure we would want to use the scratch context thingy only for > types that are really not complete in their own modules, which would > leave the scratch context method as a fairly complex, but rarely > exercised path. > > pl _______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
