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. 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'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. 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
