labath added a comment. In https://reviews.llvm.org/D49740#1188055, @jingham wrote:
> First off, I'm fine with Zachary's suggestion that we let the dust settle a > bit before we try to organize things better. We'll probably do a better job > then. Thanks. I agree this can wait, but I also do have some ideas below on what could done differently right now for the Scalar and State classes. Let me know if you think I should try that instead. (Otherwise, I'll just go ahead with this.) > But just to use these cases, for instance, Scalar is the base of how we > realize values in the debugger (ValueObject lives on top of this). It would > be good if its landing place was such that if I was looking at how we > represent values it was logically placed for that. I think putting Scalar next to the ValueObject stuff would make sense, if the ValueObjects were the only user of this class. However, that isn't the case right now. The reason I am touching this class in the first place is that it is being used from within the RegisterValue class. This means it is more than just a ValueObject helper, but more like a self-contained "utility" which can be reused in different contexts. Now, I am not actually sure whether using Scalar within RegisterValue is a good idea (seems a bit overkill for that), but it seemed to work, so I didn't want to disturb that (and I do believe that Scalar could be useful outside of the ValueObject hierarchy). However, I can take another look at what it would take to stop using Scalar in the context of RegisterValues, which would free us up to move RegisterValue without touching the Scalar class. > State is part of how we present the Process current state, so it seems like > it should be grouped with other process related entities. State is a bit funny. It is currently used from both liblldb and lldb-server, but these use different hierarchies for "process-related entities", so that's why I'm moving it to a common place. However, I can certainly see a case for LLGS and liblldb having different State enums. Sharing it doesn't buy us much (ATM it's just the StateAsCString function), and using a different enum in LLGS would have another benefit. Right now it only uses a subset of all State values, so using an different enum would allow us to capture the possible states more precisely. Doing that instead of moving State.h around should be easy enough. > And RegisterValue belongs with the other parts of lldb that take apart the > machine level state of a process. I am not sure which parts are *you* thinking about here, but I think it would be nice to have this class together with all the definitions of RegisterInfo structs and associated constants. These are now mostly in Plugin/Process/Utility, but so is a lot of other stuff. At one point I will get around to worrying about those too, so maybe then we could move all of these things to a separate module. > It will probably go away, but FastDemangle really belongs around the language > handling code. I agree that Utility is an odd place for CompletionRequest... > > SafeMachO is weird. It gets used in Host - both common & macosx, and we're > trying not to include out of Plugins so the MachO object file plugin > directory doesn't seem right. Maybe Host is a better place, it's reason for > being is so you can include both llvm/Support/MachO.h and mach/machine.h, so > even though it's not directly a host functionality it look a bit like that. > Not sure. I think Host makes sense. None of the other code (except the NativeProcessXXX classes, I guess) should really be including OS-specific stuff, so there shouldn't be a need (in an ideal world, I don't know how far we are from it right now) for this header anywhere except Host code. > There's also tension between "things that belong together functionally" and > "things that need to be separated because we want to layer one strictly on > top of the other, since we seem to be treating directories as a > representation of dependencies. Do we want to have a Values directory with > ValueObject at the top level, and Scalar in a no-dependency subdirectory? > > TTTT, if I need to find a file these days, I either Cmd-Click on a type to go > to its definition, or type Cmd-Shift-O and start typing some bits of the file > name and Xcode finds it for me. I'm pretty familiar with the code, so I > don't need a higher level skeleton to help me figure out what all is in this > project. I think at this point many of us are in this state... > > But once you get past the build issues, the real audience for this level of > organization is folks coming new to the project. It would be good to hear > from some of the newer folks what they found confusing or what would have > helped them navigate around the project as they are starting out. Yes, that's certainly a good point. I'd like to hear that too. https://reviews.llvm.org/D49740 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
