Well, I haven't had a chance to try out the changes yet, so I guess I'll reserve judgement for tomorrow. Ultimately if everything works, no harm no foul. Although as I mentioned in the earlier resposne, we don't have Windows buildbots yet.
In any case, I would love to see a more active code reviewing culture in LLDB someday. They're a source of great benefit for everyone. On Thu Dec 04 2014 at 5:36:19 PM Sean Callanan <scalla...@apple.com> wrote: > Zachary, > > It’s not clear to me that this is actually a bigger deal for non-OS X > platforms than the IOHandler change. > For one thing, unless a Platform specifically signs up for this, it does > nothing. > The idea is specifically to start this off on OS X and then have other > platforms adopt it as they are ready. To that end, I sent a summary of > what’s going in as an e-mail for people to read as they look at the commits. > > The only risk here is that we’ll break someone’s build in a way we didn’t > anticipate – which is the point of buildbots. > > Sean > > On Dec 4, 2014, at 5:33 PM, Zachary Turner <ztur...@google.com> wrote: > > Yes, but for starters we dont have a windows build bot yet (ive been > working on that), and secondly buildbots shouldnt be used as a way to > bypass the review process. > > For example, Kates change to IOHandler recently is small compared to this > one, but it still was looked at by people prior to comitting. > > It's possible these changes won't break anything, but still it doesn't > seem unreasonable to ask that particularly huge changes like this at least > have a day or two of being made public before going in. > On Thu, Dec 4, 2014 at 5:25 PM <jing...@apple.com> wrote: > >> I'm confused. Isn't that what the build bots are for? Seems better to >> rely on the immediate feedback you get from build bots for "other host" >> breakages than waiting around to see if anybody is going to grab your patch >> and apply it and try to build, etc... >> >> Jim >> >> > On Dec 4, 2014, at 5:18 PM, Zachary Turner <ztur...@google.com> wrote: >> > >> > Would it be possible to put patches up for review for at least a couple >> days BEFORE comitting? There's a pretty good chance this will break a build >> somewhere, and an average chance that the break won't be that easy to fix. >> And having broken builds for a couple of days doesn't seem reasonable. >> > >> > Im not sure how thoroughly we can review it, but at least we can >> prevent broken builds this way. >> > On Thu, Dec 4, 2014 at 5:10 PM Sean Callanan <scalla...@apple.com> >> wrote: >> > Rationale >> > >> > A historical pain point in the expression parser has been using >> functions for which the type is not completely known from debug information. >> > This is the case e.g. for functions from the C standard library, and >> also for Objective-C methods in frameworks. >> > It’s also unfortunate when handy types, enums, etc. are invisible just >> because your DWARF doesn’t happen to contain them. >> > I’m doing something about that, for OS X – but it should generalize to >> other platforms. >> > >> > Clang modules: the basics >> > >> > The code I’m about to commit adds support for Clang modules to the >> expression parser. Clang modules are described in much more detail here: >> > http://clang.llvm.org/docs/Modules.html >> > but here is a short introduction: >> > >> > A group of header files is encapsulated in a module, which is provided >> with a module.map file. >> > On OS X, this module.map is typically inside the corresponding >> framework. >> > Clang reads module.map and the corresponding headers and produces a >> compiled module. >> > This compiled module is essentially a .pch; it provides full >> information about all APIs, types, etc. defined in the module. >> > >> > LLDB support >> > >> > In order to use Clang modules, LLDB must: >> > >> > • Know where they live (from Clang’s perspective, this is the >> “sysroot”) and what compilation flags to use when parsing them; >> > • Know which ones the user wants; >> > • Compile them with its built-in Clang instance; and >> > • Use the information found in the compiled modules as >> appropriate. >> > >> > LLDB accomplishes each of these goals in the following ways: >> > >> > • Platforms can now return the appropriate Clang flags that tell >> the module importer where to find modules for the current platform. >> Explicit support is enabled in PlatformDarwin. Other platforms can opt >> into this by returning true from Platform::SupportsModules() and adding the >> appropriate compilation options when Platform::AddClangModuleCompil >> ationOptions() is called. If they return false, there should be no >> change in behavior. >> > • LLDB adds preprocessor callbacks to Clang that catch @import >> directives. When such a directive is found, LLDB directs its built-in >> Clang to import the named module. >> > • Modules are imported into a separate compiler instance (with >> its own AST context) encapsulated in ClangModulesDeclVendor so that we can >> be careful about what we actually import into expressions. Information in >> DWARF will often take precedence, for instance. >> > • ClangExpressionDeclMap – the code responsible for finding >> entities Clang asks about while it parses expressions – is being extended >> to load information from modules as appropriate. The initial commit simply >> searches for functions (e.g., printf()) but I will be adding functionality >> rapidly. >> > >> > What the upcoming patches do >> > >> > • ClangModulesDeclVendor.h/.cpp implements the portions of LLDB >> responsible for driving the modules compiler. This should be >> platform-generic code, although in practice we may need to tweak it to make >> sure it is flexible enough to handle everything. >> > • TypeVendor has been changed to DeclVendor, so that the >> Objective-C runtime can share the same method signatures with the Clang >> module importer. Places that used TypeVendor now use DeclVendor, and I’ve >> tweaked the APIs for getting types from decls to make this transition >> smooth. >> > • Platform (as mentioned above) now can return the flags >> necessary to tell Clang where modules live. PlatformDarwin has a bunch of >> new code to find these; I also have a default implementation that you can >> try out if you want, but unless you know what a module.map is I would hold >> off on this. >> > • The LLDB bundle on Mac OS X will now also include the >> compiler-specific headers Clang requires to compile standard library >> headers (e.g., tgmath.h, stdarg.h). Host can find the location of these >> headers; on non-OS X hosts, we’ll need to put them in some sensible place. >> > • ClangExpressionParser.cpp now sets up the appropriate context >> to intercept @import directives. >> > • ClangExpressionDeclMap.cpp now searches modules (if available) >> for functions if it doesn’t find them in DWARF. >> > • Targets now vend their ClangModulesDeclVendors as appropriate. >> > >> > Timeline and priorities >> > >> > I’m going to start committing today, but if any of these commits breaks >> anything for you, please let me know. >> > My priority is getting this working on OS X first. If parts of my code >> look platform-myopic, please let me know, though – I really want to see >> debugging with modules working on other platforms too. >> > Of course if any of this breaks any build, let me know immediately and >> I’ll get on it. >> > _______________________________________________ >> > lldb-dev mailing list >> > lldb-dev@cs.uiuc.edu >> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev >> > _______________________________________________ >> > lldb-dev mailing list >> > lldb-dev@cs.uiuc.edu >> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev >> >> >
_______________________________________________ lldb-dev mailing list lldb-dev@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
